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
Attachments:
fix-typo-in-comment.patchapplication/octet-stream; name=fix-typo-in-comment.patchDownload+1-1
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
Attachments:
modify-comments-for-pgstat-flush-cb-functions.patchapplication/octet-stream; name=modify-comments-for-pgstat-flush-cb-functions.patchDownload+8-8
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
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