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.

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(exit-code): determination logic #199

Merged
merged 1 commit into from
Oct 31, 2016
Merged

fix(exit-code): determination logic #199

merged 1 commit into from
Oct 31, 2016

Conversation

sarbbottam
Copy link
Owner

@sarbbottam sarbbottam commented Oct 30, 2016

existing logic to determine exit code was not correct.

argv.error is true by default
if --no-error is passed argv.error will be false
if -n was passed it would set argv.error to false

fixes #194


@ljharb @ta2edchimp

Please excuse me for the plethora of PRs over the weekend. Sorry about it.
Hopefully, this is the last one.

@codecov-io
Copy link

Current coverage is 100% (diff: 100%)

Merging #199 into master will not change coverage

@@           master   #199   diff @@
====================================
  Files           9      9          
  Lines         228    231     +3   
  Methods         0      0          
  Messages        0      0          
  Branches        0      0          
====================================
+ Hits          228    231     +3   
  Misses          0      0          
  Partials        0      0          

Powered by Codecov. Last update 55781a2...e532a6b

@sarbbottam
Copy link
Owner Author

@ta2edchimp @ljharb please review when you get time. This should get merged ASAP.

Copy link
Collaborator

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this already have a test covering it? If so, great, if not, can this PR add one?

// if --no-error is passed argv.error will be false
// if -n was passed it would set argv.error to false
if (argv.n) {
argv.error = false
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you may prefer not to mutate argv here - ie, const showError = argv.error && !argv.n; and then const processExitCode = argv.u && showError ? 1 : 0;?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally did it in that way, but though for this scope it was not required. But good practice is always good practice. Will update it accordingly.

I named the variable as errorOut.

argv.error is true by default
if --no-error is passed argv.error will be false
if -n was passed it would set argv.error to false
@sarbbottam
Copy link
Owner Author

@sarbbottam sarbbottam merged commit aa9b2cb into master Oct 31, 2016
@sarbbottam sarbbottam deleted the fix/error-out branch October 31, 2016 05:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--no-error still outputs errors
3 participants