-
Notifications
You must be signed in to change notification settings - Fork 6
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
Suppress Errors Optionally #33
Conversation
* Lets some errors bubble up so we can handle them if needed
try { | ||
await client.issues.addLabels(labelData); | ||
} catch (error) { | ||
core.setFailed(error.message); | ||
process.exit(1); | ||
} |
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.
any particular reason on removing this try...catch
block?
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.
the thought was that these errors are already caught in main, and since we added more specific error handling in there it was nice to have the errors bubble up and be handled in main.
@Anks @loftykhanna can probably merge this PR |
There probably isn't a way for us to test these PRs anymore since we do not use Pivotal anymore. Suggestions? |
@hammerdr Thanks for raising this PR! 👋🏽 WRT the comment above, we would really appreciate if you could post a few screenshots after enabling/disabling this option on one of your test repos and showing the description maybe? Feel free to blur out any confidential information from your company. |
cc @pturley
Resolves #32