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.
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.
participants (6)
-
Antoine Pitrou -
Brett Cannon -
Nick Coghlan -
Paul Moore -
Serhiy Storchaka -
Terry Reedy