Re: [PATCH] Function to get size of asynchronous notification queue

On Fri, 26 Jun 2015 at 06:03 Gurjeet Singh <gurjeet(at)singh(dot)im> wrote:

> Patch reviewed following the instructions on
> https://wiki.postgresql.org/wiki/Reviewing_a_Patch
>
>
Thank you for your review, Gurjeet.

> s/proportion/fraction/
>

I think of these as synonymous -- do you have any particular reason to
prefer "fraction"? I don't feel strongly about it either way, so I'm quite
happy to go with fraction if folks find that more expressive.

>
> + * The caller must hold (at least) shared AysncQueueLock.
>
> A possibly better wording: The caller must hold AysncQueueLock in (at
> least) shared mode.
>

Yes, that is more accurate.

>
> Unnecessary whitespace changes in pg_proc.h for existing functions.
>
>
I did group the asynchronous notification functions together, which seemed
reasonable as there are now three of them, and changed the tabbing between
the function name and namespace ID to match, as is done elsewhere in
pg_proc.h. I think those changes improve readability, but again I don't
feel strongly about it.

+DESCR("get the current usage of the asynchronous notification queue");
>
> A possibly better wording: get the fraction of the asynchronous
> notification queue currently in use
>

I have no objections to your wording.

Cheers,
BJ