-
Notifications
You must be signed in to change notification settings - Fork 151
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
feat: Add support for throwing instead of process.exit(1) on errors #118
Conversation
When using the package as a library, it's a good idea to throw an error instead of exiting with code 1. This way the user can recover from the mistake somehow (e.g. deleting a throwaway branch on during continuous integration).
I don't know how to configure the CONTENTFUL_INTEGRATION_* on travis. But the tests pass for me! |
Hi @charlespwd, |
Hi @Khaledgarbaya, @axe312ger. Any updates on this? We'd like to use this here at ALDO group. Thanks. |
Hi @charlespwd I will check this this first thing tomorrow. |
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.
LGTM
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.
Loogs good but we need tests to make sure this behaves as expected and stays that way :)
Hi @charlespwd, |
Hey @Khaledgarbaya, the e2e tests fail for me on master. Is that normal?
|
So I figured out why.
|
OK, I got an e2e test in. Let me know if you need anything else :) |
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.
Tested the test locally and it works fine.
Thanks, @charlespwd.
available in |
Summary
When using the package as a library, it's a good idea to throw an error
instead of exiting with code 1. This way the user can recover from the
mistake somehow (e.g. deleting a throwaway environment on during continuous
integration).
This seems like it solves #111 too.
Description
runMigration
export, make the function throw instead of process.exit(1).default
export, proceed as before by running process.exit(1) on errors.Motivation and Context
I'm making continuous integration scripts that creates and deletes throwaway branches on CircleCI to test that the migration scripts are valid before they are merged. When the migration fails, I would like it to delete the newly created branch. This is currently impossible because this library will
process.exit(1)
for me.Actual run-migration code:
Without this PR, the code above is unable to execute the
finally
block after errors.I'm open to feedback / suggestions on how to improve this.