On Tue, Dec 16, 2014 at 10:26 AM, Michael Paquier <michael.paqu...@gmail.com> wrote: > In any case, I have a couple of comments about this patch as-is: > - If the move to src/bin is done, let's update the MSVC scripts accordingly > - contrib/pg_rewind/.gitignore should be cleaned up from its unnecessary > entries > - README is incorrect, it is still mentioned for example that this > code should be copied inside PostgreSQL code tree as contrib/pg_rewind (Sorry email got sent...) - Code is going to need a brush to clean up things of this type: + PG_9.4_201403261 + printf("Report bugs to https://github.com/vmware/pg_rewind.\n"); - Versioning should be made the Postgres-way, aka not that: +#define PG_REWIND_VERSION "0.1" But a way similar to other utilities: pg_rewind (PostgreSQL) 9.5devel - Shouldn't we use $(SHELL) here at least? +++ b/contrib/pg_rewind/launcher @@ -0,0 +1,6 @@ +#!/bin/bash +# +# Normally, psql feeds the files in sql/ directory to psql, but we want to +# run them as shell scripts instead. + +bash I doubt that this would work for example with MinGW. - There are a couple of TODO items which may be good to fix: + * + * TODO: This assumes that there are no timeline switches on the target + * cluster after the fork. + */ and: + /* + * TODO: move old file out of the way, if any. And perhaps create the + * file with temporary name first and rename in place after it's done. + */ - Regression tests, which have a good coverage btw are made using shell scripts. There is some initialization process that could be refactored IMO as this code is duplicated with pg_upgrade. For example, listen_addresses initialization has checks fir MINGW, environment variables PG* are unset, etc. Regards, -- Michael
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers