Check if on default branch before uploading database #563
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aeisenberg
reviewed
Jun 16, 2021
Comment on lines
+704
to
+706
| const eventJson = JSON.parse( | ||
| fs.readFileSync(getRequiredEnvParam("GITHUB_EVENT_PATH"), "utf-8") | ||
| ); |
There was a problem hiding this comment.
A bit late on this comment, but I'd suggest wrapping this in a try catch block, just in case the file doesn't exist or it's malformed.
There was a problem hiding this comment.
Not a huge deal as the file is supposed to be there, but this is just for safety.
There was a problem hiding this comment.
Good point, although what would it be able to do in this case? I think we'd be limited to just throwing an error with a better message, which is certainly still a valid thing to do.
There was a problem hiding this comment.
It might also be valid to return false, but probably safer to throw.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(Note this is merging into a feature branch, and not into
main)We only want to upload a CodeQL database if analyzing the default branch, and this will avoid unnecessary uploads and help ensure we always have a recent database uploaded. For now we don't care about other branches and don't want them polluting the cache of databases for the default branch.
I initially thought we would have to check this on the github side, but turns out we can easily find out the default branch of the repository from the action.
I have tested this by running this branch of the action from a branch, a PR, and on master, and confirmed the behaviour in each case.