Skip to content

Don't download artifacts for repos with no results in MRVA#2736

Merged
charisk merged 4 commits intomainfrom
charisk/dont-download-no-results
Aug 22, 2023
Merged

Don't download artifacts for repos with no results in MRVA#2736
charisk merged 4 commits intomainfrom
charisk/dont-download-no-results

Conversation

@charisk
Copy link
Copy Markdown
Contributor

@charisk charisk commented Aug 22, 2023

Since we know the number of results, we use that to decide whether we want download an artifact or not.

Please review this commit by commit, since the only significant change is the first commit.

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

@charisk charisk requested review from a team as code owners August 22, 2023 13:29
Copy link
Copy Markdown
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.

The changes to repoHasDownloadableArtifact LGTM, and I tested it and it seems to be working as expected.

I'm not sure how to validate the changes to the mock API responses, but I trust you they are correct.

@charisk charisk merged commit 81924af into main Aug 22, 2023
@charisk charisk deleted the charisk/dont-download-no-results branch August 22, 2023 15:23
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