Postgres ReviewFest Patch: URI connection string support for libpq

Lists: pgsql-hackers
From: Nick Roosevelt <nroose(at)gmail(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Postgres ReviewFest Patch: URI connection string support for libpq
Date: 2012-01-13 04:07:04
Message-ID: CAAyg8k0KxT6xa2ADcZwJzBTdR-BeE1tN_UKbt24HvCaL3VvGig@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Just reviewed the patch for adding URI connection string support for libpg.

There seem to be many tabs in the patch. Perhaps the indentation is not
correct.

Also, the patch did not run correctly:

patching file src/interfaces/libpq/fe-connect.c
Hunk #1 succeeded at 282 with fuzz 1.
Hunk #2 FAILED at 300.
Hunk #3 FAILED at 583.
Hunk #4 FAILED at 3742.
Hunk #5 FAILED at 4132.
Hunk #6 FAILED at 4279.
Hunk #7 FAILED at 4455.
6 out of 7 hunks FAILED -- saving rejects to file
src/interfaces/libpq/fe-connect.c.rej

Seems like the file has changed since this patch was created. Please
recreate the patch.

Thanks,

Nick Roosevelt


From: Greg Smith <greg(at)2ndQuadrant(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Postgres ReviewFest Patch: URI connection string support for libpq
Date: 2012-01-13 13:28:45
Message-ID: 4F10318D.8040002@2ndQuadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 01/12/2012 11:07 PM, Nick Roosevelt wrote:
> Just reviewed the patch for adding URI connection string support for
> libpg.
> ...Seems like the file has changed since this patch was created.
> Please recreate the patch.

Thanks for the review. In the CommitFest appliation, it looks like
someone marked this patch "Returned with Feedback" after adding your
review. Whoever did that, it wasn't the right next step for this one.
"Returned with Feedback" suggests a patch has been pushed out of the
CommitFest as not possible to commit yet. It's a final state--normally
a submission going there is finished with, until the next CommitFest
starts. In a case like this one, where a problem is identified but
there's still time to fix it, the right next step is "Waiting on Author".

There's some more details about this at
http://wiki.postgresql.org/wiki/Running_a_CommitFest , which we don't
expect everyone to know because it isn't one of the popular pages to
read. I've sorted out the problem for this one already, just letting
whoever made that change know about the process here.

--
Greg Smith 2ndQuadrant US greg(at)2ndQuadrant(dot)com Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com


From: Josh Berkus <josh(at)agliodbs(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Postgres ReviewFest Patch: URI connection string support for libpq
Date: 2012-01-13 16:44:26
Message-ID: 4F105F6A.9030404@agliodbs.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


> There's some more details about this at
> http://wiki.postgresql.org/wiki/Running_a_CommitFest , which we don't
> expect everyone to know because it isn't one of the popular pages to
> read. I've sorted out the problem for this one already, just letting
> whoever made that change know about the process here.

Yeah, I need to go through the new reviewer's comments. Several of them
got ahead of the group and started marking things up on the CF page;
should be easily cleaned up though.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


From: Alex Shulgin <ash(at)commandprompt(dot)com>
To: Nick Roosevelt <nroose(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Postgres ReviewFest Patch: URI connection string support for libpq
Date: 2012-01-19 18:18:37
Message-ID: 877h0ntv2a.fsf@ash@commandprompt.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


Nick Roosevelt <nroose(at)gmail(dot)com> writes:

I am sorry, seems like my new MUA was misconfigured so the two previous
attempts to reply to -hackers@ failed. So here goes again.

> Just reviewed the patch for adding URI connection string support for
> libpg.

Thank you for taking your time on this.

> There seem to be many tabs in the patch. Perhaps the indentation is
> not correct.

I believe tabs for indentation is the project's standard.

> Also, the patch did not run correctly:
> patching file src/interfaces/libpq/fe-connect.c
> Hunk #1 succeeded at 282 with fuzz 1.
> Hunk #2 FAILED at 300.
> Hunk #3 FAILED at 583.
> Hunk #4 FAILED at 3742.
> Hunk #5 FAILED at 4132.
> Hunk #6 FAILED at 4279.
> Hunk #7 FAILED at 4455.
> 6 out of 7 hunks FAILED -- saving rejects to file
> src/interfaces/libpq/fe-connect.c.rej
> Seems like the file has changed since this patch was created. Please
> recreate the patch.

I've just tried to apply the original patch against a clean checkout of
master branch and it applies without a problem (no hunk failed, no
fuzziness.)

Could you please try again on a clean master branch?

--
Regards,
Alex