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

adding --timeout option #339

Merged
2 commits merged into from
Apr 17, 2017
Merged

adding --timeout option #339

2 commits merged into from
Apr 17, 2017

Conversation

ghost
Copy link

@ghost ghost commented Apr 17, 2017

In a new test I used done() from a callback to have cleaner error handling/output when an error is expected but the spawn finishes successfully. It felt dirtier having the .catch() have to check if it was the error we expect or another error.

Copy link
Owner

@raineorshine raineorshine left a comment

Choose a reason for hiding this comment

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

Looks good! One small correction.

Also, please add the new option to the README, under advanced options.

Thanks!

bin/ncu Outdated
@@ -31,6 +31,7 @@ program
.option('-r, --registry <url>', 'specify third-party npm registry')
.option('-s, --silent', "don't output anything (--loglevel silent)")
.option('-t, --greatest', 'find the highest versions available instead of the latest stable versions')
.option('--timeout <seconds>', 'set a global timeout in ms')
Copy link
Owner

Choose a reason for hiding this comment

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

This should say --timeout <ms>

Copy link
Author

Choose a reason for hiding this comment

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

Good catch. I started out using seconds. Thanks :)

@ghost ghost merged commit e86d728 into master Apr 17, 2017
@ghost ghost deleted the global-timeout branch April 17, 2017 20:57
This pull request was closed.
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.

1 participant