Re: [python-committers] Please edit the commit message when merge a PR
11.07.17 19:39, Brett Cannon пише: There's isn't a way to block a merge at that stage. But one thing I've been thinking about is adding a check to Bedevere post-merge that sees if the commit message was left unchanged (not quite sure if I can come up with a reliable heuristic, though). In instances where the committers forgot, Bedevere would simply leave a message saying something like, "Hey, thanks for taking the time to merge a PR, but please don't forget to clean up the commit message before merging." Basically a friendly reminder to not forget next time (I'm also thinking of doing something similar for the formatting of the PR title after merging). +1 for adding a post-merge check and a friendly reminder. I still see merged commits containing messages from all intermediate changes. I'm afraid that it would be rough from my side to remind about this every time, but the bot is impartial. ___ 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, 10 Jul 2017 at 07:57 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) > > 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. > There's isn't a way to block a merge at that stage. But one thing I've been thinking about is adding a check to Bedevere post-merge that sees if the commit message was left unchanged (not quite sure if I can come up with a reliable heuristic, though). In instances where the committers forgot, Bedevere would simply leave a message saying something like, "Hey, thanks for taking the time to merge a PR, but please don't forget to clean up the commit message before merging." Basically a friendly reminder to not forget next time (I'm also thinking of doing something similar for the formatting of the PR title after merging). ___ 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
2017-07-10 17:35 GMT+02:00 Guido van Rossum : > 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. Oh, I noticed that comments were hidden after a rebase or squash. But I never understood the link between squash/rebase and hidden comments. Ok, it makes sense to prefer merge and multiple commits to workaround UI limitations of GitHub PRs :-) 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/
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/
