John Naylor <jcnaylor@gmail.com> writes:
> Version 8, rebased against 76b6aa41f41d.
I took a preliminary look through this, without yet attempting to execute
the script against HEAD. I have a few thoughts:
* I'm inclined not to commit the conversion scripts to the repo. I doubt
there are third parties out there with a need for them, and if they do
need them they can get 'em out of this thread in the mailing list
archives. (If anyone has a different opinion about that, speak up!)
* I notice you have a few "preliminary cleanup" changes here and there
in the series, such as fixing the inconsistent spelling of
Anum_pg_init_privs_initprivs. These could be applied before we start
the main conversion process, and I'm inclined to do that since we could
get 'em out of the way early. Ideally, the main conversion would only
touch the header files and related scripts/makefiles.
* I'm a little disturbed by the fact that 0002 has to "re-doublequote
values that are macros expanded by initdb.c". I see that there are only
a small number of affected places, so maybe it's not really worth working
harder, but I worry that something might get missed. Is there any way to
include this consideration in the automated conversion, or at least to
verify that we found all the places to quote? Or, seeing that 0004 seems
to be introducing some quoting-related hacks to genbki.pl anyway, maybe
we could take care of the issue there?
* In 0003, I'd recommend leaving the re-indentation to happen in the next
perltidy run (assuming perltidy would fix that, which I think is true but
I might be wrong). It's just creating more review work to do it here.
In any case, the patch summary line is pretty misleading since it's
*not* just reindenting, but also refactoring genbki.pl. (BTW, if that
refactoring would work on the script as it is, maybe that's another
thing we could do early? The more we can do before "flag day", the
better IMO.)
* In 0006, I'm not very pleased with the introduction of
"Makefile.headers". I'd keep those macros where they are in
catalog/Makefile. backend/Makefile doesn't need to know about that,
especially since it's doing an unconditional invocation of
catalog/Makefile anyway. It could just do something like
submake-schemapg:
$(MAKE) -C catalog generated-headers
and leave it to catalog/Makefile to know what needs to happen for
both schemapg.h and the other generated files.
Overall, though, this is looking pretty promising.
regards, tom lane
