On Thu, Oct 13, 2016 at 5:53 AM, Victor Wagner <vitus@wagner.pp.ru> wrote:
> On Thu, 13 Oct 2016 12:30:59 +0530
> Mithun Cy <mithun.cy@enterprisedb.com> wrote:
>> On Fri, Sep 30, 2016 at 2:14 PM, Victor Wagner <vitus@wagner.pp.ru>
>> wrote:
>> Okay but for me consistency is also important. Since we agree to
>> disagree on some of the comments and others have not expressed any
>> problem I am moving it to committer.
>
> Thank you for your efforts improving my patch
I haven't deeply reviewed this patch, but on a quick read-through it
certainly seems to need a lot of cleanup work.
+ <varlistentry id="libpq-connect-hostorder" xreflabel="hostorder">
+ <term><literal>hostorder</literal></term>
+ <listitem>
I don't think that putting everything at the same indentation level
matches the style of the surrounding documentation.
@@ -985,7 +1042,6 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname </para> </listitem> </varlistentry>
- <varlistentry id="libpq-connect-connect-timeout"
xreflabel="connect_timeout"> <term><literal>connect_timeout</literal></term> <listitem>
Spurious hunk.
+ /* Host has explicitely specified port */
Spelling. This same error is repeated elsewhere.
+ /* Check if port is numeric */
+ for (nptr=r+1;nptr<q;nptr++)
+ {
+ if (*nptr<'0' ||*nptr >'9')
Formatting. Consider using pgindent to get this into correct PostgreSQL style.
+ * behavoir.
Spelling.
+ * the beginning (and putting its addres into given pointer.
Spelling.
+ * Returns 1 if address is choosen and 0 if there are no more addresses
Spelling.
+ * Initialize random number generator in case if nobody have done it
+ * before. Use value from rand along with time in case random number
Grammar ("in case nobody has done it already"). Also, is it really OK
for libpq to call srand() as a side effect? I think that seems rather
unfriendly.
+ * taget_server_type is set to 'any' or is not exlicitely
Multiple spelling mistakes.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
