Re: [PATCH] Refactor pgbench to make future improvements easier - Mailing list pgsql-hackers

From Zsolt Parragi
Subject Re: [PATCH] Refactor pgbench to make future improvements easier
Date
Msg-id CAN4CZFMtnNJvz1eY+zSvPQLYMoJqmQP31iDijeAX3daUYuBMuw@mail.gmail.com
Whole thread
In response to [PATCH] Refactor pgbench to make future improvements easier  (Hannu Krosing <hannuk@google.com>)
Responses Re: [PATCH] Refactor pgbench to make future improvements easier
List pgsql-hackers
Hello!

From the email explaining that this is a split, and the commit
messages saying "extract" I expected a move-only patchset that
strictly extracts part of the code into different files/modules. But
that doesn't seem to be the case?

For example "executeMetaCommand" looks completely different in the
end, chooseScript and other functions got signature changes, the
number of comment lines got reduces by ~25%, a VariableScopeStack
typedef is introduced that isn't used anywhere, etc.

I think for something like this to be easily reviewable, moves and
logic changes should be strictly separate patches, not mixed together,
and anything that's not a simple cut-and-paste into another file
should be mentioned. In the current patchset, 0007 seems to be the
closest to a pure move, but even that isn't just that. (When I am
doing something similar, I usually follow a one refactoring - one
commit/patch approach during the review, only squashing things
together later)

I also checked the commit history of pgbench, it seems to get around
~4 backpatched commits per year, so that doesn't seem to be that bad
through a refactoring.



pgsql-hackers by date:

Previous
From: Andrew Dunstan
Date:
Subject: Re: Add ASCII fast path to Unicode normalization functions
Next
From: Tom Lane
Date:
Subject: Re: Coverage with make coverage-html is broken on latest Debian using lcov v2