-
-
Notifications
You must be signed in to change notification settings - Fork 213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Share the stderr with the user, when non-zero exit code #927
Conversation
…to non-zero exit code.
for more information, see https://pre-commit.ci
src/setuptools_scm/_run_cmd.py
Outdated
if not res and not message: | ||
message = p.stderr | ||
|
||
return Result(status=res, message=message) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets define the Result class in the same module as its only used for this particular function
it mgiht make sense to use the logging based output in any case (when has command initially was created, warnings was used as trace was only opt in by env var
now with the logging setup that chooses level, we can just log a error directly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are very fast to reply! Thank you. 😄
Just logging directly would indeed more straight forward. That will remove the need to return a complex type from has_command, so I can just skip the result wrapper entirely. I'll rework the PR.
The diff is a bit smaller now, and there is a test. 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lovely
Want to squash? |
Yes please! |
When a has_command() returns False, we eat all the output and just say that it was not found. In case the False return is due to a non-zero exit status from a command we did actually find, this makes it quite messy to figure out what went wrong.
This is an attempt to let output from the failing command bubble up the stack, with a minimal amount of disruption to surrounding code.