Update EnableTimeoutParams timeout type comment
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.
Attachments:
v1-0001-Update-EnableTimeoutParams-timeout-type-comment.patchapplication/octet-stream; name=v1-0001-Update-EnableTimeoutParams-timeout-type-comment.patchDownload+1-2
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.
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
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.
Attachments:
v2-0001-Remove-stale-EnableTimeoutParams-type-comment.patchapplication/octet-stream; name=v2-0001-Remove-stale-EnableTimeoutParams-type-comment.patchDownload+1-2
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
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.