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:

1) 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.
2) 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.
3) 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.
4) 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/