On 10/22/12 11:47 AM, Phil Sorber wrote: > On Sun, Oct 21, 2012 at 6:20 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: >> Phil Sorber <p...@omniti.com> writes: >>> Here is the new patch. I renamed the utility from pg_ping to pingdb to >>> go along with the naming convention of src/bin/scripts. >> >> Uh, no, that's not a step forward. Leaving out a "pg" prefix from those >> script names is universally agreed to have been a mistake. We've not >> felt that changing the legacy names is worth the amount of pain it'd >> cause, but that doesn't mean that we should propagate the mistake into >> brand new executable names. >> >> regards, tom lane > > Ok. I asked about this and got no response so I assume there were no > strong opinions. I have reverted to the pg_ping name. Patches > attached.
Quick review ... Code: *************** install: all installdirs *** 54,59 **** --- 55,61 ---- $(INSTALL_PROGRAM) clusterdb$(X) '$(DESTDIR)$(bindir)'/clusterdb$(X) $(INSTALL_PROGRAM) vacuumdb$(X) '$(DESTDIR)$(bindir)'/vacuumdb$(X) $(INSTALL_PROGRAM) reindexdb$(X) '$(DESTDIR)$(bindir)'/reindexdb$(X) + $(INSTALL_PROGRAM) pg_ping$(X) '$(DESTDIR)$(bindir)'/pg_ping$(X) installdirs: $(MKDIR_P) '$(DESTDIR)$(bindir)' Whitespace misaligned + exit(3); // Return UNKNOWN No // comments. + while (NULL != conn_opt_ptr->keyword) Better writte as while (conn_opt_ptr->keyword != NULL) or while (conn_opt_ptr->keyword) Also, it seems that about 75% of the patch is connection options processing. How about we get rid of all that and just have them specify a connection string? It would be a break with tradition, but maybe it's time for something new. Functionality: I'm missing the typical ping functionality to ping continuously. If we're going to call it pg_ping, it ought to do something similar to ping, I think. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers