Re: [python-committers] Please edit the commit message when merge a PR
On 7/10/2017 10:49 AM, Berker Peksağ wrote: On Mon, Jul 10, 2017 at 4:14 PM, Serhiy Storchaka wrote: When a PR is consistent on several commits, the final commit message composed by GitHub contains messages of all these commits: "fix typo", "address yyy comments", "revert zzz". If the initial commit message contained errors (e.g. absent issue number), it is easy to edit the title and text of a PR, but the initial commit message lefts unchanged. GitHub allows to edit the commit message of squashed commit, and please don't ignore this possibility. Otherwise the commit message in the repository will be ugly if not worse. +1! (and thank you for writing this email, Serhiy) Ditto. I can't think of a way to automatically prevent a PR from merging if body of the squashed commit contains "fix typo" commits. I think this is a pretty annoying problem and perhaps we should ask contributors to squash multiple commits themselves even if we continue to use the "squash and merge" option. When people did that, we asked them not to. When reviewing, I want to be able to see both commits in response to comments and the total diff (by clicking files at the top). If people push a squash merge after a PR is approved both by CI and core reviewer, they might make a mistake. In any case, another round of CI is triggered. In any case, I find editing easy. I often delete all and copy from the new blurb. When I write a patch myself, I try to write at least some commit messages that should remain in the final result. So I might copy some commit messages into the blurb. ___ 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/
Re: [python-committers] Please edit the commit message when merge a PR
Often the committer has more context to write a proper commit message, and asking the contributor to do the squash is just wasting time (plus in general we *don't* want contributors to squash, since that loses the context for the review). So I'm with Sergey. This is how we do it in the mypy-related projects. On Mon, Jul 10, 2017 at 8:09 AM, Barry Warsaw wrote: > On Jul 10, 2017, at 04:57 PM, Victor Stinner wrote: > > >I would prefer to ask the author to squash and/or rebase his/her > >commits rather than having to edit the commit message myself. I prefer > >that the commit message is part of the review, and not only done by > >the one who clicks on the Merge button. > > > >It would prefer mistakes in the commit message. > > > >GitHub PR UI is not ideal to review commit messages :-/ > > GitLab is roughly the same, and I always edit commit messages when squash > merging. While ideally the author could do this, I think sensible commit > messages are more important, so +1 for having the committer edit them at > their > discretion. > > -Barry > ___ > 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/ > -- --Guido van Rossum (python.org/~guido) ___ 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/
Re: [python-committers] Please edit the commit message when merge a PR
On Jul 10, 2017, at 04:57 PM, Victor Stinner wrote: >I would prefer to ask the author to squash and/or rebase his/her >commits rather than having to edit the commit message myself. I prefer >that the commit message is part of the review, and not only done by >the one who clicks on the Merge button. > >It would prefer mistakes in the commit message. > >GitHub PR UI is not ideal to review commit messages :-/ GitLab is roughly the same, and I always edit commit messages when squash merging. While ideally the author could do this, I think sensible commit messages are more important, so +1 for having the committer edit them at their discretion. -Barry ___ 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/
Re: [python-committers] Please edit the commit message when merge a PR
I would prefer to ask the author to squash and/or rebase his/her commits rather than having to edit the commit message myself. I prefer that the commit message is part of the review, and not only done by the one who clicks on the Merge button. It would prefer mistakes in the commit message. GitHub PR UI is not ideal to review commit messages :-/ Victor 2017-07-10 15:14 GMT+02:00 Serhiy Storchaka : > When a PR is consistent on several commits, the final commit message > composed by GitHub contains messages of all these commits: "fix typo", > "address yyy comments", "revert zzz". If the initial commit message > contained errors (e.g. absent issue number), it is easy to edit the title > and text of a PR, but the initial commit message lefts unchanged. GitHub > allows to edit the commit message of squashed commit, and please don't > ignore this possibility. Otherwise the commit message in the repository will > be ugly if not worse. > > ___ > 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/ ___ 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/
Re: [python-committers] Please edit the commit message when merge a PR
On Mon, Jul 10, 2017 at 4:14 PM, Serhiy Storchaka wrote: > When a PR is consistent on several commits, the final commit message > composed by GitHub contains messages of all these commits: "fix typo", > "address yyy comments", "revert zzz". If the initial commit message > contained errors (e.g. absent issue number), it is easy to edit the title > and text of a PR, but the initial commit message lefts unchanged. GitHub > allows to edit the commit message of squashed commit, and please don't > ignore this possibility. Otherwise the commit message in the repository will > be ugly if not worse. +1! (and thank you for writing this email, Serhiy) I can't think of a way to automatically prevent a PR from merging if body of the squashed commit contains "fix typo" commits. I think this is a pretty annoying problem and perhaps we should ask contributors to squash multiple commits themselves even if we continue to use the "squash and merge" option. ___ 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/
[python-committers] Please edit the commit message when merge a PR
When a PR is consistent on several commits, the final commit message composed by GitHub contains messages of all these commits: "fix typo", "address yyy comments", "revert zzz". If the initial commit message contained errors (e.g. absent issue number), it is easy to edit the title and text of a PR, but the initial commit message lefts unchanged. GitHub allows to edit the commit message of squashed commit, and please don't ignore this possibility. Otherwise the commit message in the repository will be ugly if not worse. ___ 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/