On Sun, Dec 2, 2012 at 5:56 AM, Phil Sorber
<phil@omniti.com> wrote:
Here is the updated patch. I renamed it, but using v5 to stay consistent.
After looking at this patch, I found the following problems:
- There are a couple of whitespaces still in the code, particularly at the end of those lines
+ const char *pguser = NULL;
+ const char *pgdbname = NULL;
- When describing the values that are returned by pg_isready, it is awkward to refer to the returning values as plain integers as those values are part of an enum. You should add references to PQPING_OK, PQPING_REJECT, PQPING_NO_RESPONSE and PQPING_NO_ATTEMPT instead. Also add to reference links in the docs redirecting to them, with things of the type <xref linkend="libpq-pqpingparams-pqping-ok"> or related.
- Same thing with this example:
+ <para>
+ Standard Usage:
+ <screen>
+ <prompt>$</prompt> <userinput>pg_isready</userinput>
+ <prompt>$</prompt> <userinput>echo $?</userinput>
+ <computeroutput>0</computeroutput>
+ </screen>
+ </para>
For the time being PQPING_OK returns 0 because it is on top of the enum PGPing, but this might change if for a reason or another the order of outputs is changed.
Once those things are fixed, I think this will be ready for committer review as everybody here seem to agree with your approach.
--
Michael Paquier
http://michael.otacoo.com
