Skip to content
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

Add override for code scanning analysis of default branch #1603

Merged
merged 1 commit into from
Mar 23, 2023

Conversation

charisk
Copy link
Contributor

@charisk charisk commented Mar 23, 2023

Provide an override for code scanning analyses of the default branch.

See internal linked issue for details.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.
  • Confirm the changelog has been updated if necessary.

@charisk charisk requested a review from a team as a code owner March 23, 2023 11:15
Copy link
Contributor

@marcogario marcogario left a comment

Choose a reason for hiding this comment

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

LGTM, but I'd appreciate more 👀

@@ -630,6 +630,10 @@ function removeRefsHeadsPrefix(ref: string): string {
// Is the version of the repository we are currently analyzing from the default branch,
// or alternatively from another branch or a pull request.
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor:

// Returns whether we are analyzing the default branch for the repository.
// For cases where the repository information might not be available (e.g., dynamic workflows), this can be forced by the environment variable CODE_SCANNING_IS_ANALYZING_DEFAULT_BRANCH.

Copy link
Contributor

@henrymercer henrymercer left a comment

Choose a reason for hiding this comment

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

Looks good. A minor comment in addition to Marco's doc suggestion.

Comment on lines 230 to 233
process.env["CODE_SCANNING_IS_ANALYZING_DEFAULT_BRANCH"] = "true";
t.deepEqual(await actionsutil.isAnalyzingDefaultBranch(), true);

process.env["CODE_SCANNING_IS_ANALYZING_DEFAULT_BRANCH"] = "false";
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we move these tests up, or into a separate test function, such that we're testing actionsutil.isAnalyzingDefaultBranch without event.repository.default_branch existing?

@charisk charisk dismissed a stale review via ffb4aa2 March 23, 2023 13:22
@charisk charisk force-pushed the charisk/default-branch-analayzing-override branch from 4147f0b to ffb4aa2 Compare March 23, 2023 13:22
@charisk charisk dismissed a stale review via 07d5a56 March 23, 2023 13:26
@charisk charisk force-pushed the charisk/default-branch-analayzing-override branch from ffb4aa2 to 07d5a56 Compare March 23, 2023 13:26
@charisk charisk force-pushed the charisk/default-branch-analayzing-override branch from 07d5a56 to 94cc1de Compare March 23, 2023 13:31
Copy link
Contributor

@henrymercer henrymercer left a comment

Choose a reason for hiding this comment

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

Nice!

@charisk
Copy link
Contributor Author

charisk commented Mar 23, 2023

Thanks for the reviews!

@charisk charisk merged commit 0214d1d into main Mar 23, 2023
@charisk charisk deleted the charisk/default-branch-analayzing-override branch March 23, 2023 14:21
@github-actions github-actions bot mentioned this pull request Mar 27, 2023
6 tasks
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.

3 participants