Horton

Thread

Commits

  1. Remove stale comment

  1. Update EnableTimeoutParams timeout type comment

    Xuneng Zhou <xunengzhou@gmail.com> — 2026-07-01T01:43:40Z

    Hi Hackers,

While reading the timeout code, I noticed that the comment for
EnableTimeoutParams timeout type is kinda stale. Currently, we support
three types of timeout TMPARAM_AFTER/AT/EVERY, but the comment only
mentions the first two. It may not cause confusion, since the two
structs are adjacent, but it might still be worth making this clearer.
Attached a patch to update it.

-- 
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.

  2. Re: Update EnableTimeoutParams timeout type comment

    Xuneng Zhou <xunengzhou@gmail.com> — 2026-07-01T01:49:55Z

    On Wed, Jul 1, 2026 at 9:43 AM Xuneng Zhou <xunengzhou@gmail.com> wrote:
>
> Hi Hackers,
>
> While reading the timeout code, I noticed that the comment for
> EnableTimeoutParams timeout type is kinda stale. Currently, we support
> three types of timeout TMPARAM_AFTER/AT/EVERY, but the comment only
> mentions the first two. It may not cause confusion, since the two
> structs are adjacent, but it might still be worth making this clearer.
> Attached a patch to update it.

Opus, it should be "the struct and the enum", not two structs.

-- 
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.

  3. Re: Update EnableTimeoutParams timeout type comment

    John Naylor <johncnaylorls@gmail.com> — 2026-07-01T04:01:04Z

    On Wed, Jul 1, 2026 at 8:44 AM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> While reading the timeout code, I noticed that the comment for
> EnableTimeoutParams timeout type is kinda stale. Currently, we support
> three types of timeout TMPARAM_AFTER/AT/EVERY, but the comment only
> mentions the first two. It may not cause confusion, since the two
> structs are adjacent, but it might still be worth making this clearer.
> Attached a patch to update it.

It seems pointless to have a comment whose entire content is to list
every current enum member of that type. I'd just delete the comment,
since it's likely to just going to get outdated again. If any other
objects of type enum have this, they should get the same treatment.

-- 
John Naylor
Amazon Web Services

  4. Re: Update EnableTimeoutParams timeout type comment

    Xuneng Zhou <xunengzhou@gmail.com> — 2026-07-01T04:41:27Z

    Hi John,

On Wed, Jul 1, 2026 at 12:01 PM John Naylor <johncnaylorls@gmail.com> wrote:
>
> On Wed, Jul 1, 2026 at 8:44 AM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> > While reading the timeout code, I noticed that the comment for
> > EnableTimeoutParams timeout type is kinda stale. Currently, we support
> > three types of timeout TMPARAM_AFTER/AT/EVERY, but the comment only
> > mentions the first two. It may not cause confusion, since the two
> > structs are adjacent, but it might still be worth making this clearer.
> > Attached a patch to update it.
>
> It seems pointless to have a comment whose entire content is to list
> every current enum member of that type. I'd just delete the comment,
> since it's likely to just going to get outdated again. If any other
> objects of type enum have this, they should get the same treatment.

Agreed. The comment just duplicates nearby code and can easily go
stale again. Removing it also makes sense to me. Here's the patch with
the treatment.

--
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.

  5. Re: Update EnableTimeoutParams timeout type comment

    John Naylor <johncnaylorls@gmail.com> — 2026-07-02T06:23:01Z

    On Wed, Jul 1, 2026 at 11:41 AM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> On Wed, Jul 1, 2026 at 12:01 PM John Naylor <johncnaylorls@gmail.com> wrote:
> > It seems pointless to have a comment whose entire content is to list
> > every current enum member of that type. I'd just delete the comment,
> > since it's likely to just going to get outdated again. If any other
> > objects of type enum have this, they should get the same treatment.
>
> Agreed. The comment just duplicates nearby code and can easily go
> stale again. Removing it also makes sense to me. Here's the patch with
> the treatment.

My last sentence was meant to imply that I'd rather find and remove
all such comments, rather than fix only one and leave the rest for
someone else to stumble across. I had Claude whip up a throwaway
script to find candidate useless comments, and didn't see any others
like it, only those documenting a value that can have only a subset of
the enum values.

I've pushed your patch.

--
John Naylor
Amazon Web Services

  6. Re: Update EnableTimeoutParams timeout type comment

    Xuneng Zhou <xunengzhou@gmail.com> — 2026-07-02T07:02:38Z

    On Thu, Jul 2, 2026 at 2:23 PM John Naylor <johncnaylorls@gmail.com> wrote:
>
> On Wed, Jul 1, 2026 at 11:41 AM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> > On Wed, Jul 1, 2026 at 12:01 PM John Naylor <johncnaylorls@gmail.com> wrote:
> > > It seems pointless to have a comment whose entire content is to list
> > > every current enum member of that type. I'd just delete the comment,
> > > since it's likely to just going to get outdated again. If any other
> > > objects of type enum have this, they should get the same treatment.
> >
> > Agreed. The comment just duplicates nearby code and can easily go
> > stale again. Removing it also makes sense to me. Here's the patch with
> > the treatment.
>
> My last sentence was meant to imply that I'd rather find and remove
> all such comments, rather than fix only one and leave the rest for
> someone else to stumble across. I had Claude whip up a throwaway
> script to find candidate useless comments, and didn't see any others
> like it, only those documenting a value that can have only a subset of
> the enum values.

Sorry, I didn't get that. Thanks for investigating this.

> I've pushed your patch.

Thanks.

-- 
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.