From: | Alex Shulgin <ash(at)commandprompt(dot)com> |
---|---|
To: | Harold Giménez <harold(dot)gimenez(at)gmail(dot)com> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Reviewing patch "URI connection string support for libpq" |
Date: | 2012-02-24 13:52:51 |
Message-ID: | 878vjsxrrg.fsf@commandprompt.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Harold Giménez <harold(dot)gimenez(at)gmail(dot)com> writes:
> I have interest in the URI connection string support patch[1], so I'm
> in the process of reviewing it. I have a couple of comments and
> questions:
Hello, thank you for your interest and the review!
> 1. I see no tests in the patch. I'd like to start getting together a
> set of tests, likely based on the connection string permutations found
> on Greg Smith's response[2]. However I don't find an obvious place to
> put them. They could possibly live in the test/examples directory.
> Another thought is to use dblink in a test, although it may be
> problematic to depend on a contrib package for a test, to say the
> least. Any thoughts on how to test this are most welcome.
I was also curious on how to add any sort of regression testing (likely
using psql,) but didn't get any advice as far as I can recall.
> 2. The documentation/manual was not updated as part of this patch, so
> this is pending.
I've marked the patch as Work-In-Progress and specifically omitted
documentation changes until we settle on functionality.
> 3. I for one do prefer the `postgres` prefix, as opposed to
> `postgresql` for the reasons stated on an earlier thread [3]. In my
> opinion the best way to move forward is to support them both.
The updated v4 version of the patch does cover this:
http://archives.postgresql.org/pgsql-hackers/2012-02/msg01141.php
--
Alex
From | Date | Subject | |
---|---|---|---|
Next Message | Yeb Havinga | 2012-02-24 14:17:12 | Re: [v9.2] Add GUC sepgsql.client_label |
Previous Message | Marko Kreen | 2012-02-24 13:52:10 | Let's drop V2 protocol |