-
Notifications
You must be signed in to change notification settings - Fork 991
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
Output failure message only once #273
Conversation
@nexdrew yes, consider the code review enqueued -- sorry that I've been so AWOL lately., can't wait to put some time into OSS. |
@bcoe No worries. 🌴 |
@@ -28,17 +28,22 @@ module.exports = function (yargs, y18n) { | |||
return self | |||
} | |||
|
|||
var outputFailure = true |
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.
I'd be tempted to start the variable in a false state, just a personal preference when I'm using a variable as a flag. perhaps: failureOutput
= false
, and set it to true
once you spit out an error -- just a minor nit feel free to ignore.
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.
I thought of that, but then I noticed showHelpOnFail
above defaults to true
, so I figured it depended on the use-case. I also thought if (outputFailure)
was easier to read/follow than if (!something)
. But I'm happy to change it if you'd like.
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.
Changed with commit e61d536.
this is great |
I will merge once the build passes with the last commit. |
Output failure message only once
Fixes issue #270 (concerning when
.exitProcess(false)
is used) by implementing the simplest solution mentioned in comment there.Includes and satisfies test specs from PR #269.