Re: [Python-Dev] bpo-5001: More-informative multiprocessing error messages (#3079)

30.08.17 01:52, Antoine Pitrou пише:
https://github.com/python/cpython/commit/bd73e72b4a9f019be514954b1d40e64dc3a... commit: bd73e72b4a9f019be514954b1d40e64dc3a5e81c branch: master author: Allen W. Smith, Ph.D <drallensmith@users.noreply.github.com> committer: Antoine Pitrou <pitrou@free.fr> date: 2017-08-30T00:52:18+02:00 summary:
bpo-5001: More-informative multiprocessing error messages (#3079)
* Make error message more informative
Replace assertions in error-reporting code with more-informative version that doesn't cause confusion over where and what the error is.
* Additional clarification + get travis to check
* Change from SystemError to TypeError
As suggested in PR comment by @pitrou, changing from SystemError; TypeError appears appropriate.
* NEWS file installation; ACKS addition (will do my best to justify it by additional work)
* Making current AssertionErrors in multiprocessing more informative
* Blurb added re multiprocessing managers.py, queues.py cleanup
* Further multiprocessing cleanup - went through pool.py
* Fix two asserts in multiprocessing/util.py
* Most asserts in multiprocessing more informative
* Didn't save right version
* Further work on multiprocessing error messages
* Correct typo
* Correct typo v2
* Blasted colon... serves me right for trying to work on two things at once
* Simplify NEWS entry
* Update 2017-08-18-17-16-38.bpo-5001.gwnthq.rst
* Update 2017-08-18-17-16-38.bpo-5001.gwnthq.rst
OK, never mind.
* Corrected (thanks to pitrou) error messages for notify
* Remove extraneous backslash in docstring.
Please, please don't forget to edit commit messages before merging. An excessively verbose commit message will be kept in the repository forever and will harm future developers that read a history.

On Wed, 30 Aug 2017 08:48:56 +0300 Serhiy Storchaka <storchaka@gmail.com> wrote:
30.08.17 01:52, Antoine Pitrou пише:
https://github.com/python/cpython/commit/bd73e72b4a9f019be514954b1d40e64dc3a... commit: bd73e72b4a9f019be514954b1d40e64dc3a5e81c branch: master author: Allen W. Smith, Ph.D <drallensmith@users.noreply.github.com> committer: Antoine Pitrou <pitrou@free.fr> date: 2017-08-30T00:52:18+02:00 summary:
bpo-5001: More-informative multiprocessing error messages (#3079)
* Make error message more informative
Replace assertions in error-reporting code with more-informative version that doesn't cause confusion over where and what the error is.
* Additional clarification + get travis to check
* Change from SystemError to TypeError
As suggested in PR comment by @pitrou, changing from SystemError; TypeError appears appropriate.
* NEWS file installation; ACKS addition (will do my best to justify it by additional work)
* Making current AssertionErrors in multiprocessing more informative
* Blurb added re multiprocessing managers.py, queues.py cleanup
* Further multiprocessing cleanup - went through pool.py
* Fix two asserts in multiprocessing/util.py
* Most asserts in multiprocessing more informative
* Didn't save right version
* Further work on multiprocessing error messages
* Correct typo
* Correct typo v2
* Blasted colon... serves me right for trying to work on two things at once
* Simplify NEWS entry
* Update 2017-08-18-17-16-38.bpo-5001.gwnthq.rst
* Update 2017-08-18-17-16-38.bpo-5001.gwnthq.rst
OK, never mind.
* Corrected (thanks to pitrou) error messages for notify
* Remove extraneous backslash in docstring.
Please, please don't forget to edit commit messages before merging. An excessively verbose commit message will be kept in the repository forever and will harm future developers that read a history.
Sorry, I routinely forget about it. Can we have an automated check for this? Regards Antoine.

On 30 August 2017 at 19:39, Antoine Pitrou <solipsis@pitrou.net> wrote:
On Wed, 30 Aug 2017 08:48:56 +0300 Serhiy Storchaka <storchaka@gmail.com> wrote:
Please, please don't forget to edit commit messages before merging. An excessively verbose commit message will be kept in the repository forever and will harm future developers that read a history.
Sorry, I routinely forget about it. Can we have an automated check for this?
Not while we're pushing the "squash & merge" button directly, as there's no opportunity for any automated hooks to run at that point :( More options open up if the actual commit is being handled by a bot, but even that would still depend on us providing an updated commit message via some mechanism. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia

On 30 August 2017 at 10:48, Nick Coghlan <ncoghlan@gmail.com> wrote:
On 30 August 2017 at 19:39, Antoine Pitrou <solipsis@pitrou.net> wrote:
On Wed, 30 Aug 2017 08:48:56 +0300 Serhiy Storchaka <storchaka@gmail.com> wrote:
Please, please don't forget to edit commit messages before merging. An excessively verbose commit message will be kept in the repository forever and will harm future developers that read a history.
Sorry, I routinely forget about it. Can we have an automated check for this?
Not while we're pushing the "squash & merge" button directly, as there's no opportunity for any automated hooks to run at that point :(
More options open up if the actual commit is being handled by a bot, but even that would still depend on us providing an updated commit message via some mechanism.
We could have a check for PRs that contain multiple commits. The check would flag that the commit message needs manual editing before merging, and would act as a prompt for submitters who are willing to do so to squash their PRs themselves. Personally, I'd prefer that as, apart from admin activities like making sure the PR and issue number references are present and correct, I think the original submitter has a better chance of properly summarising the PR in the merged commit message anyway. Paul

On Wed, 30 Aug 2017 at 02:56 Paul Moore <p.f.moore@gmail.com> wrote:
On 30 August 2017 at 10:48, Nick Coghlan <ncoghlan@gmail.com> wrote:
On 30 August 2017 at 19:39, Antoine Pitrou <solipsis@pitrou.net> wrote:
On Wed, 30 Aug 2017 08:48:56 +0300 Serhiy Storchaka <storchaka@gmail.com> wrote:
Please, please don't forget to edit commit messages before merging. An excessively verbose commit message will be kept in the repository forever and will harm future developers that read a history.
Sorry, I routinely forget about it. Can we have an automated check for this?
Not while we're pushing the "squash & merge" button directly, as there's no opportunity for any automated hooks to run at that point :(
More options open up if the actual commit is being handled by a bot, but even that would still depend on us providing an updated commit message via some mechanism.
We could have a check for PRs that contain multiple commits. The check would flag that the commit message needs manual editing before merging, and would act as a prompt for submitters who are willing to do so to squash their PRs themselves. Personally, I'd prefer that as, apart from admin activities like making sure the PR and issue number references are present and correct, I think the original submitter has a better chance of properly summarising the PR in the merged commit message anyway.
So you would want a comment when the PR reaches "awaiting merge" with instructions requesting the author do their own squash commit to simplify the message for us? There's also https://github.com/python/bedevere/issues/14 to help post-commit remind core devs when they forgot to do something. While it would be too late to fix a problem, hopefully regular reminders would still be helpful.

On 31 August 2017 at 20:27, Brett Cannon <brett@python.org> wrote:
So you would want a comment when the PR reaches "awaiting merge" with instructions requesting the author do their own squash commit to simplify the message for us?
That would work. It could say that the PR consists of multiple commits and the commit message needs to be summarised when merging, and suggest that if the submitter is willing, they could squash and summarise the message themselves. I don't want to give the impression we're insisting the submitter squash, rather we're pointing out there's an additional step needed and the submitter can help by doing that step if they wish.
There's also https://github.com/python/bedevere/issues/14 to help post-commit remind core devs when they forgot to do something. While it would be too late to fix a problem, hopefully regular reminders would still be helpful.
Yeah, that would probably be useful for regular committers. Paul

On 8/31/2017 3:27 PM, Brett Cannon wrote:
On Wed, 30 Aug 2017 at 02:56 Paul Moore <p.f.moore@gmail.com
do so to squash their PRs themselves. Personally, I'd prefer that as, apart from admin activities like making sure the PR and issue number references are present and correct, I think the original submitter has a better chance of properly summarising the PR in the merged commit message anyway.
My experience is different.
So you would want a comment when the PR reaches "awaiting merge" with instructions requesting the author do their own squash commit to
My impression is that this makes the individual commits disappear.
simplify the message for us?
Not me. -- Terry Jan Reedy
participants (6)
-
Antoine Pitrou
-
Brett Cannon
-
Nick Coghlan
-
Paul Moore
-
Serhiy Storchaka
-
Terry Reedy