On Tue, Oct 23, 2012 at 6:12 PM, Peter Eisentraut <pete...@gmx.net> wrote: > 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
Fixed. > > + exit(3); // Return UNKNOWN > > No // comments. Changed. > > + while (NULL != conn_opt_ptr->keyword) > > Better writte as > > while (conn_opt_ptr->keyword != NULL) > > or > > while (conn_opt_ptr->keyword) Changed to the latter. > > > 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. I don't think that should be a part of this patch. If we think that we should remove connection params and just pass a connection string we should probably deprecate connection params and remove them everywhere together over a period of time like with other features. I don't think we should introduce a new binary that doesn't work the same way as all the other binaries. I went back and checked, and realized I didn't do it correctly, but this patch now does allow a connection string to be passed to -d. This seems to be the accepted way to do this. > > > 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. It's not named after the network utility but after the libpq function PQping. Since this doesn't output latencies or really much of anything else like the network ping, I'm not sure it makes sense to do that, but I could be convinced otherwise. Attaching an updated patch.
Description: Binary data
-- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers