Check the exit code of a command, instead of relying on stderr #432
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… this in the output. Note that this is tricky to do right. Some commands like ``git`` seem to print everything to stderr, leading us to think there are errors, but the exit code is zero, so it should be fine. We do not want to ask the user to often if she wants to continue or not. But we do not want to silently swallow important errors either.
Interestingly, this needed some fixes in the tests, where commands had gone wrong for years but we never noticed. * At the end of `fullrelease` we try to push, and this always failed because there was no push target. We now ignore the push commands and instead print them. * One test renames `CHANGES.rst` to `CHANGES.md`, and then calling `python setup.py --version` actually fails: `setup.py` still tries to read the no longer existing `CHANGES.rst`. I hope we now strike a good balance between showing error output, exit codes, and asking if the user wants to continue. With the previous alpha release, the balance was not good for me. The final `git push` used `stderr` to report that everything was fine, and then `postrelease` treated this as an error, and asked me if I wanted to continue. Now we check the exit code.
|
I tried it out with some purposefully badly configured packages; it gives errors where it should and no errors where it shouldn't, so it seems fine to me. |
|
Thanks for reviewing! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a command fails, always ask the user to continue or not. I hope with this we strike a good balance between showing error output, exit codes, and asking if the user wants to continue.
With the previous alpha release, the balance was not good for me. The final
git pushusedstderrto report that everything was fine, and thenpostreleasetreated this as an error, and asked me if I wanted to continue. Now we check the exit code.@elisallenens @gforcada Since Reinout is unavailable for a while, can you review it, or simply try this branch out when releasing some packages? I could also do another alpha release to more easily try it.