Skip to content

Always report exceptions #22

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

Closed
wants to merge 2 commits into from
Closed

Conversation

joshhale
Copy link
Contributor

@joshhale joshhale commented May 6, 2020

We currently catch exceptions in a couple of places where it would be better to let them reach the top level where they would be included in the status report from the action.

  • Don't catch exceptions during upload lower than the top level
  • Don't catch exceptions during config initialization lower than the top level
  • Correct the action name passed from the analyze action

Merge / deployment checklist

  • Run test builds as necessary. Can be on this repository or elsewhere as needed in order to test the change - please include links to tests in other repos!
    • CodeQL using init/analyze actions action run ✔️
    • 3rd party tool using upload action action run ✔️
  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.

Copy link
Contributor

@robertbrignull robertbrignull left a comment

Choose a reason for hiding this comment

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

Changes LGTM

@robertbrignull robertbrignull mentioned this pull request May 14, 2020
4 tasks
@robertbrignull
Copy link
Contributor

@joshhale, this has conflict but otherwise can be merged. Or would you like someone else to take over this PR?

@joshhale
Copy link
Contributor Author

@robertbrignull I'd love it it someone could take this over. I am pretty swamped atm.

@robertbrignull
Copy link
Contributor

Closed in favour of #49 because fixing the merge conflicts proved a bit too annoying.

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.

2 participants