Re: [python-committers] Please edit the commit message when merge a PR

[Serhiy Storchaka]

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

[Brett Cannon]

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

[Victor Stinner]

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

[Terry Reedy]

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

[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.

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

[Barry Warsaw]

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

[Victor Stinner]

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

[Berker Peksağ]

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

[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/