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

Exit 1 in case of CLI error #209

Merged
merged 7 commits into from
Mar 15, 2016
Merged

Exit 1 in case of CLI error #209

merged 7 commits into from
Mar 15, 2016

Conversation

matteofigus
Copy link
Member

Fixes #174

Now that all the cli's facades have an optional callback (not used by the cli) - it is possible, in case it is not provided, to wrap it with a utility that exits with a process.exit(1) when a callback is fired with an error.

To test locally with unix/mac:

  • checkout the branch locally
  • run a command that should fail like node oc-cli.js registry add 23456
  • run echo $? to see previous command's exit code

@matteofigus
Copy link
Member Author

Can you review this @antwhite or @jankowiakmaria or @pbazydlo ?

@pbazydlo
Copy link
Collaborator

lgtm, it's a scary one though as you're now forcing death instead of silence, won't it break anything?

@pbazydlo
Copy link
Collaborator

(outside of oc itself)

@matteofigus
Copy link
Member Author

@pbazydlo that's what we want if the callback is fired with an error. Previously, it was exiting with 0 in case of both success and fail.

Also, notice that the callback-less usage of facades is limited to cli. Grunt wrapper and possible future implementations are able to provide the optional callbacks and do different things in case of errors.

pbazydlo added a commit that referenced this pull request Mar 15, 2016
Exit 1 in case of CLI error
@pbazydlo pbazydlo merged commit 54b7312 into master Mar 15, 2016
@pbazydlo pbazydlo deleted the cli-back branch March 15, 2016 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants