On 3/21/18, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> The attached is just one incremental patch on top of your v11 series.
> I couldn't think of an easy way to migrate the changes back into the
> most relevant diffs of your series, so I didn't try.
I've applied your changes to the v12 patch series (attached), but I
hope you'll allow two nit-picky adjustments:
-s/pg_XXX.h/pg_xxx.h/ in the README. There seems to be greater
precedent for the lower-case spelling if the rest of the word is lower
case.
-I shortened the data example in the README so it would comfortably
fit on two lines. Spreading it out over three lines doesn't match
what's in the data files. It's valid syntax, but real data is
formatted to at most two lines (See rewrite_dat.pl. Hmm, maybe I
should make that more explicit elsewhere in the README)
> I also have not spent much time yet looking at the end-product .h and .dat
> files. I did note a bit of distressing inconsistency in the formatting of
> the catalog struct declarations, some of which predates this patch but it
> seems like you've introduced more. I think what we ought to standardize
> on is a format similar to this in pg_opclass.h:
>
> CATALOG(pg_opclass,2616)
> {
> /* index access method opclass is for */
> Oid opcmethod BKI_LOOKUP(pg_am);
>
Done, with blank lines interspersed. I put most changes of this sort
in with the other cleanups in patch 0004. I neglected to do this
separately for couple of tiny tables that have lookups, but no default
values. I don't think it impacts the readability of patch 0007 much.
On 3/22/18, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I don't really think it's legal C; I'd rather write BKI_DEFAULT('\054').
[snip]
>> /* One of the ATTRIBUTE_IDENTITY_* constants below, or '\0' */
>> char attidentity BKI_DEFAULT("");
>
> That definitely seems like a hack --- why not BKI_DEFAULT('\0') ?
Done (patch 0006).
Other changes:
-A README note about OID macros (patch 0007).
-A couple minor cosmetic rearrangements and comment/commit message edits.
Open items:
-Test MSVC.
-Arrange for rewrite_dat.pl to run when perltidy does.
-I was a bit cavalier about when to use =/:= in the Makefiles. Not
sure if there's a preferred project style for when the choice doesn't
really matter.
-Maybe document examples of how to do bulk-editing of data files?
-John Naylor
