On Wed, Sep 20, 2017 at 11:27 PM, Victor Stinner victor.stinner@gmail.com wrote:
Hi,
Before the GitHub era, in the old "Mercurial era", the unwritten rule was to not merge a patch written by a developer who has the commit bit, to not "steal" his/her work. The old workflow (patches attached to the bug tracker) didn't allow to easily keep the author. You had to find the author email and full name and specify it manually.
Moreover, there was a written rule of using the name of the developer who actually pushed the commit, so the commiters took the responsability of any regression (reminder the old era with no pre-commit CI? ;-)).
In the new Git era, the author and committer *can* be two different people. Examples with "git log --pretty=full":
commit 9abee722d448c1c00c7d4e11ce242ec7b13e5c49 Author: Victor Stinner victor.stinner@gmail.com Commit: GitHub noreply@github.com
commit 8f51bb436f8adfd139cad046b91cd462c7f27f6c (tag: v3.7.0a1) Author: Ned Deily nad@python.org Commit: Ned Deily nad@python.org
commit 9b47af65375fab9318e88ccb061394a36c8c6c33 Author: svelankar 17737361+svelankar@users.noreply.github.com Commit: Raymond Hettinger rhettinger@users.noreply.github.com
My question is: is someone opposed that a core developer clicks on the [Merge] button for a PR proposed by a different core developer?
Personally I would prefer if other core devs just left a positive review without merging, for a few reasons:
- before merging I want to double check that everything is fine, and possibly tweak a few minor things before merging. Some authors also push WIP or working but unpolished PRs to get some early feedback before following up with more iterations, and if they don't specify it clearly you might end up merging too early.
- even if you are not "stealing their work", you deprive them of the closure they get by finally merging a PR they have been working on for some time.
- some tools (e.g. buildbot) might only look at the committer field, and this might end up notifying the wrong person or skewing some statistics.
- I don't see a reason to do that in the first place: I expect the author to quickly merge a PR once all the checks pass (especially since they can now do it from their smart watch while lying on the beach), and even if the author takes a few days to do so, it usually doesn't really matter.
IMHO having a committer different than the author is valuable since the responsability is now shared by two developers instead of single one. It's similar to the "Signed-Off" tags used by the Linux kernel, but the list is limited to a single Signed-Off :-) Well, the committer is usually seen as the most reponsible, but now we can complain to the author as well *if needed* :-D
IME if the author is not a core dev, the responsibility falls on the committer; if the author is a core dev, the responsibility falls on the author/comitter (i.e. the core-dev). If core devs merge their own patches, the responsible will always be the committer.
If you merge other core devs' patches, you are likely to get blamed when something breaks even if the code wasn't yours, and if the author runs away and you want someone else to blame, you can always find all the reviews by looking at the PR referenced by the changet :)
Best Regards, Ezio Melotti
Victor
python-committers mailing list python-committers@python.org https://mail.python.org/mailman/listinfo/python-committers Code of Conduct: https://www.python.org/psf/codeofconduct/