Skip to content

Conversation

@mveytsman
Copy link
Contributor

This adds better messaging to tell the user what languages we aren't autobuilding and prompts them to replace this action with custom build steps if we do fail.

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/finish actions
    • 3rd party tool using upload action
  • 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.

I've explained my comment from earlier. Other than that LGTM

src/autobuild.ts Outdated

run().catch(e => {
core.setFailed("autobuild action failed: " + e);
core.setFailed("We were unable to automatically build your code. Please replace the call to the autobuild action with your custom build steps. codeql/autobuild action failed. " + e);
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry the comment has got lost now. What I was saying earlier was that this catch block won't catch what you want. You want to leave this catch as it is and add this error message to the catch on line 52. This catch block will only get errors from our code like reportActionSucceeded.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah thanks!

@mveytsman mveytsman merged commit 5218f93 into master May 1, 2020
@mveytsman mveytsman deleted the autobuild_errors branch May 8, 2020 15:22
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.

4 participants