Skip to content

Convert local query commands to typed commands#2194

Merged
koesie10 merged 5 commits intomainfrom
koesie10/typed-local-query-commands
Mar 22, 2023
Merged

Convert local query commands to typed commands#2194
koesie10 merged 5 commits intomainfrom
koesie10/typed-local-query-commands

Conversation

@koesie10
Copy link
Copy Markdown
Member

Please review this commit-by-commit. Each commit makes the move to typed commands a bit easier.

One difference from all other typed commands is that the local query commands are using a separate logger, and this is not supported by the command manager because it is quite specific to this extension. Therefore, we create a separate command manager which uses a different logger to separate the commands. I have purposefully not added this to the App interface since we're not using it anywhere yet. Instead of adding it to the ExtensionApp, we could also create it in the extension.ts file and not store it on the ExtensionApp. I'm not sure which approach is better.

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.

The local query commands are using a separate logger, and this is not
supported by the command manager because it is quite specific to this
extension. Therefore, we create a separate command manager which uses
a different logger to separate the commands.
@koesie10 koesie10 marked this pull request as ready for review March 21, 2023 12:09
@koesie10 koesie10 requested a review from a team as a code owner March 21, 2023 12:09
@koesie10 koesie10 requested a review from a team March 21, 2023 12:09
databaseManager: dbm,
queryRunner,
queryHistoryManager,
databaseManager,
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.

💯 👩‍🍳

@koesie10 koesie10 enabled auto-merge March 22, 2023 10:30
@koesie10 koesie10 merged commit 76558b8 into main Mar 22, 2023
@koesie10 koesie10 deleted the koesie10/typed-local-query-commands branch March 22, 2023 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants