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

Fix ShellError.is_not_installed() #139

Merged
merged 1 commit into from Mar 24, 2017

Conversation

Projects
None yet
3 participants
@AusIV

AusIV commented Mar 24, 2017

ShellError.is_not_installed() was broken in ce7bc0e
when it stopped running Popen with the "shell=True" argument. Since
it wasn't being run through a shell, it was throwing an OSError instead
of running but exiting with an exit code 127.

This is a minimal change so that the ShellError will still work the
same as before, but if we get an OSError we pass in the 127 exit code.
This way if there are still any scenarios where exit code 127 would
be run by the application, it will still work as before.

Fix ShellError.is_not_installed()
ShellError.is_not_installed() was broken in ce7bc0e
when it stopped running Popen with the "shell=True" argument. Since
it wasn't being run through a shell, it was throwing an OSError instead
of running but exiting with an exit code 127.

This is a minimal change so that the ShellError will still work the
same as before, but if we get an OSError we pass in the 127 exit code.
This way if there are still any scenarios where exit code 127 would
be run by the application, it will still work as before.
@AusIV

This comment has been minimized.

AusIV commented Mar 24, 2017

Addresses: #130

@AusIV

This comment has been minimized.

AusIV commented Mar 24, 2017

I guess it also addresses #132

@coveralls

This comment has been minimized.

coveralls commented Mar 24, 2017

Coverage Status

Coverage increased (+1.5%) to 91.518% when pulling 2b0e01c on AusIV:master into 4d061b4 on deanmalmgren:master.

@deanmalmgren deanmalmgren merged commit 3699ba1 into deanmalmgren:master Mar 24, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@deanmalmgren

This comment has been minimized.

Owner

deanmalmgren commented Mar 24, 2017

this is perfect, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment