| From: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> |
|---|---|
| To: | Hannu Krosing <hannuk(at)google(dot)com> |
| Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: [PATCH] Refactor pgbench to make future improvements easier |
| Date: | 2026-10-03 22:20:18 |
| Message-ID: | CAN4CZFMtnNJvz1eY+zSvPQLYMoJqmQP31iDijeAX3daUYuBMuw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | 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.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-10-03 22:26:45 | Re: Coverage with make coverage-html is broken on latest Debian using lcov v2 |
| Previous Message | Andrew Dunstan | 2026-10-03 21:33:44 | Re: Add ASCII fast path to Unicode normalization functions |