Skip to content

Use the repos defined in the query result instead of the query#1268

Merged
charisk merged 1 commit intomainfrom
charisk/repositories-from-query-result
Apr 4, 2022
Merged

Use the repos defined in the query result instead of the query#1268
charisk merged 1 commit intomainfrom
charisk/repositories-from-query-result

Conversation

@charisk
Copy link
Copy Markdown
Contributor

@charisk charisk commented Apr 4, 2022

In order to support system defined repo lists (see linked internal issue) we need to stop relying on the repository names being available to us on the query object, but instead get that information from the query result.

This PR updates a couple of uses of the repo lists to be using the query result instead of the query.

Checklist

N/A:

  • 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 a review from a team as a code owner April 4, 2022 08:09
Copy link
Copy Markdown
Contributor

@shati-patel shati-patel 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! 🌼


Just a comment about a minor behaviour change: Do we now only count repositories with results? E.g.

Before this PR:
image

vs
After this PR:
image

I don't think this is necessarily a problem, especially for security researchers running on loads of repos at once! But (in future) we might want to change the design slightly, so we're not showing the number of repos with results in two places? 🤔

Copy link
Copy Markdown
Contributor

@shati-patel shati-patel left a comment

Choose a reason for hiding this comment

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

Ignore my previous comment! That turned out to be a different issue 😅

@charisk charisk merged commit ad3565d into main Apr 4, 2022
@charisk charisk deleted the charisk/repositories-from-query-result branch April 4, 2022 10:03
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