Skip to content

Add prompt for updating GitHub databases#3079

Merged
koesie10 merged 5 commits intomainfrom
koesie10/update-github-databases
Nov 23, 2023
Merged

Add prompt for updating GitHub databases#3079
koesie10 merged 5 commits intomainfrom
koesie10/update-github-databases

Conversation

@koesie10
Copy link
Copy Markdown
Member

@koesie10 koesie10 commented Nov 20, 2023

This adds a prompt for updating GitHub databases for the current repository. If the user clicks "Update", it will prompt for which languages to update the database for. Once selected, it will download and store the new databases, and then automatically delete the old ones. It will keep the selected database to the same language and repository.

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.

@koesie10 koesie10 force-pushed the koesie10/update-github-databases branch 2 times, most recently from d434636 to d03082d Compare November 20, 2023 11:07
@koesie10 koesie10 force-pushed the koesie10/update-github-databases branch from d03082d to a0295d6 Compare November 20, 2023 13:52
Base automatically changed from koesie10/download-multiple-github-databases to main November 21, 2023 10:42
@koesie10 koesie10 marked this pull request as ready for review November 21, 2023 10:42
@koesie10 koesie10 requested review from a team as code owners November 21, 2023 10:42
@koesie10 koesie10 requested a review from starcke November 21, 2023 10:42
Copy link
Copy Markdown
Contributor

@starcke starcke 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, mostly just minor comments and thoughts.

Comment thread extensions/ql-vscode/src/databases/github-database-module.ts Outdated
const databaseUpdates = Array.from(newestExistingDatabasesByLanguage.values())
.map((newestExistingDatabase): DatabaseUpdate | null => {
const origin = newestExistingDatabase.origin;
if (origin?.type !== "github") {
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.

Isnt this already the case from the filter above? or is there some other way it can be another type?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, this is already the case. However, TypeScript doesn't carry forward that information from the filter without additional type information, which would be more error-prone and actually make it less safe. So, this case should never be hit, but it's necessary to tell TypeScript that this is still the case and that we have access to GitHub origin information.

Comment thread extensions/ql-vscode/src/databases/github-database-updates.ts
return null;
}

return {
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.

If there is no new commit, but the CodeQL tools have been upgraded since the DB was created should we then also trigger an upgrade?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, I think that is something we should consider. I've now changed the check to also consider the database's created_at date. This can result in more prompts (e.g. if there is a scheduled workflow run to scan main every day and there are no commits), but I think that's fine for now; we can always reconsider.

Comment thread extensions/ql-vscode/src/databases/github-database-module.ts Outdated
Copy link
Copy Markdown
Contributor

@starcke starcke left a comment

Choose a reason for hiding this comment

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

👍

@koesie10 koesie10 merged commit 208efb5 into main Nov 23, 2023
@koesie10 koesie10 deleted the koesie10/update-github-databases branch November 23, 2023 11:17
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