Thread: Typo in comment for pgstat_database_flush_cb()
Another thing I noticed is $SUBJECT: I think “if lock could not immediately acquired” should be “if lock could not be immediately acquired”. Attached is a patch for that. Best regards, Etsuro Fujita
Attachment
Etsuro Fujita <etsuro.fujita@gmail.com> 于2025年3月30日周日 18:28写道：
Another thing I noticed is $SUBJECT: I think “if lock could not
immediately acquired” should be “if lock could not be immediately
acquired”. Attached is a patch for that.
Agree.
The patch looks good to me.
Thanks,
Tender Wang
On 30/03/2025 13:28, Etsuro Fujita wrote: > Another thing I noticed is $SUBJECT: I think “if lock could not > immediately acquired” should be “if lock could not be immediately > acquired”. Attached is a patch for that. Yep. And there are more instances of the same typo in other such flush_cb functions, if you search for "immediately acquired". -- Heikki Linnakangas Neon (https://neon.tech)
On 30/03/2025 14:32, Heikki Linnakangas wrote: > On 30/03/2025 13:28, Etsuro Fujita wrote: >> Another thing I noticed is $SUBJECT: I think “if lock could not >> immediately acquired” should be “if lock could not be immediately >> acquired”. Attached is a patch for that. > > Yep. And there are more instances of the same typo in other such > flush_cb functions, if you search for "immediately acquired". While we're at it, the comment feels a bit awkward to me anyway. Maybe rephrase it to "If nowait is true and the lock could not be immediately acquired, returns false without flushing the entry. Otherwise returns true." -- Heikki Linnakangas Neon (https://neon.tech)
On Sun Mar 30, 2025 at 4:39 AM PDT, Heikki Linnakangas wrote: > On 30/03/2025 14:32, Heikki Linnakangas wrote: >> On 30/03/2025 13:28, Etsuro Fujita wrote: >>> Another thing I noticed is $SUBJECT: I think “if lock could not >>> immediately acquired” should be “if lock could not be immediately >>> acquired”. Attached is a patch for that. >> >> Yep. And there are more instances of the same typo in other such >> flush_cb functions, if you search for "immediately acquired". +1 for chainging other occurrences of this in other pgstat_*.c files. > While we're at it, the comment feels a bit awkward to me anyway. Maybe > rephrase it to "If nowait is true and the lock could not be immediately > acquired, returns false without flushing the entry. Otherwise returns true." This rephrasing does sounds better. Best regards, Gurjeet http://Gurje.et
On Sun, Mar 30, 2025 at 7:54 PM Gurjeet Singh <gurjeet@singh.im> wrote: > On Sun Mar 30, 2025 at 4:39 AM PDT, Heikki Linnakangas wrote: > > On 30/03/2025 14:32, Heikki Linnakangas wrote: > >> On 30/03/2025 13:28, Etsuro Fujita wrote: > >>> Another thing I noticed is $SUBJECT: I think “if lock could not > >>> immediately acquired” should be “if lock could not be immediately > >>> acquired”. Attached is a patch for that. > >> > >> Yep. And there are more instances of the same typo in other such > >> flush_cb functions, if you search for "immediately acquired". > > +1 for chainging other occurrences of this in other pgstat_*.c files. > > > While we're at it, the comment feels a bit awkward to me anyway. Maybe > > rephrase it to "If nowait is true and the lock could not be immediately > > acquired, returns false without flushing the entry. Otherwise returns true." > > This rephrasing does sounds better. +1 for both suggestions. So I modified the comment as such in each file with such a flush_cb function. I will push the patch. Thanks Heikki for the suggestions! Thanks Gurjeet and Tender for the review! Best regards, Etsuro Fujita
Attachment
On Sun, Apr 06, 2025 at 01:31:50PM +0200, Etsuro Fujita wrote: > +1 for both suggestions. So I modified the comment as such in each > file with such a flush_cb function. I will push the patch. Thanks for the fix. Could you also backpatch that down to v15? It would be good to keep this level of comment documentation consistent across all branches. -- Michael
Attachment
On Sun, Apr 6, 2025 at 11:44 PM Michael Paquier <michael@paquier.xyz> wrote: > On Sun, Apr 06, 2025 at 01:31:50PM +0200, Etsuro Fujita wrote: > > +1 for both suggestions. So I modified the comment as such in each > > file with such a flush_cb function. I will push the patch. > > Thanks for the fix. Could you also backpatch that down to v15? It > would be good to keep this level of comment documentation consistent > across all branches. Sure, I will do that as well. Best regards, Etsuro Fujita
On Mon, Apr 7, 2025 at 6:50 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote: > On Sun, Apr 6, 2025 at 11:44 PM Michael Paquier <michael@paquier.xyz> wrote: > > Could you also backpatch that down to v15? It > > would be good to keep this level of comment documentation consistent > > across all branches. > > Sure, I will do that as well. Pushed and back-patched. It took me much longer than expected to return to this. Thanks! Best regards, Etsuro Fujita