Re: Commitfest problems
От | Mark Cave-Ayland |
---|---|
Тема | Re: Commitfest problems |
Дата | |
Msg-id | 548DE400.8020908@ilande.co.uk обсуждение исходный текст |
Ответ на | Re: Commitfest problems (Peter Geoghegan <pg@heroku.com>) |
Ответы |
Re: Commitfest problems
(Robert Haas <robertmhaas@gmail.com>)
|
Список | pgsql-hackers |
On 14/12/14 18:24, Peter Geoghegan wrote: > On Sun, Dec 14, 2014 at 9:05 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> TBH, I'm not really on board with this line of argument. I don't find >> broken-down patches to be particularly useful for review purposes. An >> example I was just fooling with this week is the GROUPING SETS patch, >> which was broken into three sections for no good reason at all. (The >> fourth and fifth subpatches, being alternative solutions to one problem, >> are in a different category of course.) Too often, decisions made in >> one subpatch don't make any sense until you see the larger picture. > > It sounds like they didn't use the technique effectively, then. I > think it will be useful to reviewers that I've broken out the > mechanism through which the ON CONFLICT UPDATE patch accepts the > EXCLUDED.* pseudo-alias into a separate commit (plus docs in a > separate commit, as well as tests) - it's a non-trivial additional > piece of code that it wouldn't be outrageous to omit in a release, and > isn't particularly anticipated by earlier cumulative commits. Maybe > people don't have a good enough sense of what sort of subdivision is > appropriate yet. I think that better style will emerge over time. For me this is a great example of how to get more developers involved in patch review. Imagine that I'm an experienced developer with little previous exposure to PostgreSQL, but with a really strong flex/bison background. If someone posts a patch to the list as a single grouping sets patch, then I'm going to look at that and think "I have no way of understanding this" and do nothing with it. However if it were posted as part of patchset with a subject of "[PATCH] gram.y: add EXCLUDED pseudo-alias to bison grammar" then this may pique my interest enough to review the changes to the grammar rules, with the barrier for entry reduced to understanding just the PostgreSQL-specific parts. What should happen over time is that developers like this would earn the trust of the committers, so committers can spend more time reviewing the remaining parts of the patchset. And of course the project has now engaged a new developer into the community who otherwise may not have not participated. > Of course, if you still prefer a monolithic commit, it's pretty easy > to produce one from a patch series. It's not easy to go in the other > direction, though. Agreed. ATB, Mark.
В списке pgsql-hackers по дате отправления: