Re: Making C function declaration parameter names consistent with corresponding definition names

On Sun, Sep 18, 2022 at 9:07 PM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> I'm slightly confused about "still not close to being commitable"
> along with "this is now the sixth and final patch.". That seems to
> imply that you're not planning to send any more patches but you don't
> think this is commitable. I'm assuming I've misunderstood that.

I meant that the "big patch" now has a new order -- it is sixth/last in
the newly revised patchset, v3. I don't know how many more patch
revisions will be required, but at least one or two more revisions
seem likely.

Here is the stuff that it less ready, or at least seems ambiguous:

1. The pg_dump patch is relatively opinionated about how to resolve
inconsistencies, and makes quite a few changes to the .c side.

Seeking out the "lesser inconsistency" resulted in more lines being
changed. Maybe you won't see it the same way (maybe you'll prefer the
other trade-off). That's just how it ended up.

2. The same thing is true to a much smaller degree with the jsonb patch.

3. The big patch itself is...well, very big. And written on autopilot,
to a certain degree. So that one just needs more careful examination,
on general principle.

> Looking at 0004 I see a few issues:
>
> 1. ConnectDatabase() seems to need a bit more work in the header
> comment. There's a reference to AH and AHX. The parameter is now
> called "A".
>
> 2. setup_connection() still references AH->use_role in the comments
> (line 1102). Similar problem on line 1207 with AH->sync_snapshot_id
>
> 3. setupDumpWorker() still makes references to AH->sync_snapshot_id
> and AH->use_role in the comments. The parameter is now called "A".
>
> 4. dumpSearchPath() still has a comment which references AH->searchpath

Will fix all those in the next revision. Thanks.

--
Peter Geoghegan