Skip to content

Add support for system defined repository lists#1271

Merged
charisk merged 7 commits intomainfrom
charisk/system-defined-repo-lists
Apr 6, 2022
Merged

Add support for system defined repository lists#1271
charisk merged 7 commits intomainfrom
charisk/system-defined-repo-lists

Conversation

@charisk
Copy link
Copy Markdown
Contributor

@charisk charisk commented Apr 4, 2022

Adds support for system defined repository lists (top 10, top 100, etc.)

image

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.

* @param repoSelection The selection to check.
* @returns A boolean flag indicating if the selection is valid or not.
*/
export function isInvalidSelection(repoSelection: RepositorySelection): boolean {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

minor: I find the double negative here a little harder to parse mentally. Could you change this to isValidSelection and invert all the booleans?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Great idea! Done.

Comment thread extensions/ql-vscode/src/remote-queries/repository-selection.ts
Comment thread extensions/ql-vscode/src/remote-queries/repository-selection.ts
@aeisenberg
Copy link
Copy Markdown
Contributor

I like this approach.

Base automatically changed from charisk/move-remote-queries-test-files to main April 5, 2022 07:40
@charisk charisk marked this pull request as ready for review April 5, 2022 08:30
@charisk charisk requested review from a team as code owners April 5, 2022 08:30
Copy link
Copy Markdown
Contributor

@aeisenberg aeisenberg 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. Two comments, which you can take or leave.

Comment thread extensions/ql-vscode/src/remote-queries/repository-selection.ts Outdated
Comment thread extensions/ql-vscode/src/remote-queries/repository-selection.ts Outdated
Co-authored-by: Andrew Eisenberg <aeisenberg@github.com>
@charisk charisk merged commit 22ed090 into main Apr 6, 2022
@charisk charisk deleted the charisk/system-defined-repo-lists branch April 6, 2022 08:05
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