Skip to content

Move convertGithubNwoToDatabaseUrl to databases/github-databases/api.ts#3453

Merged
robertbrignull merged 1 commit intomainfrom
robertbrignull/convertGithubNwoToDatabaseUrl
Mar 11, 2024
Merged

Move convertGithubNwoToDatabaseUrl to databases/github-databases/api.ts#3453
robertbrignull merged 1 commit intomainfrom
robertbrignull/convertGithubNwoToDatabaseUrl

Conversation

@robertbrignull
Copy link
Copy Markdown
Contributor

(Builds upon the #3452 PR because otherwise it'll just conflict. The changes aren't related except that the functions being moved are next to each other.)

I argue that the convertGithubNwoToDatabaseUrl function (and promptForLanguage which it uses) aren't particularly about fetching or importing databases and are more about interacting with the GitHub API, though I do realise the distinction isn't particularly clear.

These functions also stand out from the rest of database-fetcher.ts because they don't require arguments like DatabaseManager, storagePath: string, or CodeQLCliServer, while almost all of the other functions do.

I think it's neater to pull these functions out and move them to databases/github-databases/api.ts instead. This puts them next to listDatabases which makes sense as they largely just act on the output of that function.

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.

@robertbrignull robertbrignull requested a review from a team as a code owner March 7, 2024 16:43
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 sensible! Thanks 🧹

Base automatically changed from robertbrignull/findDirWithFile to main March 11, 2024 10:57
@robertbrignull robertbrignull merged commit b436fbd into main Mar 11, 2024
@robertbrignull robertbrignull deleted the robertbrignull/convertGithubNwoToDatabaseUrl branch March 11, 2024 10:58
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