Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Allow windows exit codes. Fixes #48 #49

Closed
wants to merge 3 commits into from
Closed

Allow windows exit codes. Fixes #48 #49

wants to merge 3 commits into from

Conversation

stefanjcollier
Copy link

@stefanjcollier stefanjcollier commented Dec 14, 2017 •
edited by di
Loading

Fixes #48.

@stefanjcollier stefanjcollier mentioned this pull request Dec 14, 2017
Closed
@stefanjcollier
Copy link
Author

Ah I missed the tests!

@di
Copy link
Owner

di commented Dec 14, 2017

I think we could simplify this a bit by doing something like:

try:
  OK = os.EX_OK
  ...
except AttributeError:  # On a non-unix system
  OK = 0
  ...

That way we always have the exit codes set, even if the windows detection fails (or, hypothetically, if it just isn't a Windows or a Unix platform).

@stefanjcollier
Copy link
Author

Sure, I'll do that now

@di
Copy link
Owner

di commented Dec 21, 2017

Hey @stefanjcollier, just wanted to see if you had any problems or questions about wrapping this PR up. Feel free to discuss here if so.

di pushed a commit that referenced this pull request Feb 23, 2018
@di
Fixes tests for fix #49
13e612c
@di di mentioned this pull request Feb 23, 2018
Merged
@di di closed this in #55 Feb 23, 2018
di added a commit that referenced this pull request Feb 23, 2018
@di
Non-unix exit codes and non-multiprocess exit code fix (#55)
fdeffb7 
* Allow windows exit codes. Fixes #48

* Fixes tests for fix #49

* change final os.EX_OK

* Simplify exit codes

* Correct exit codes when vladiate fails

* Move tests

* Don't lint ./dist
@XiaoyuZeng XiaoyuZeng mentioned this pull request Jul 15, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@stefanjcollier @di