Skip to content

Split flows for running remote queries#1749

Merged
koesie10 merged 5 commits intomainfrom
koesie10/split-remote-query-flows
Nov 11, 2022
Merged

Split flows for running remote queries#1749
koesie10 merged 5 commits intomainfrom
koesie10/split-remote-query-flows

Conversation

@koesie10
Copy link
Copy Markdown
Member

This removes the runRemoteQuery method and instead moves all logic specific to remote queries/variant analysis to the remote queries manager and variant analysis manager respectively. This will make it easier to completely remove the remote queries manager in the future.

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.

This removes the `runRemoteQuery` method and instead moves all logic
specific to remote queries/variant analysis to the remote queries
manager and variant analysis manager respectively. This will make it
easier to completely remove the remote queries manager in the future.
@koesie10 koesie10 requested review from a team as code owners November 11, 2022 09:49
Comment thread extensions/ql-vscode/src/remote-queries/remote-queries-manager.ts Outdated
}

async function runRemoteQueriesApiRequest(
export async function runRemoteQueriesApiRequest(
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.

Can this and buildRemoteQueryEntity be moved to the RemoteQueriesManager too?

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, these can be moved out of this file. I'd prefer to not place them into the RemoteQueriesManager itself since it doesn't depend on the state of that class, but I'll move the buildRemoteQueryEntity into remote-query.ts and create a new file for this method in remote-queries-api.ts.

The `runRemoteQuery` and `runVariantAnalysis` were returning values
which were only used in tests. This removes them and replaces the tests
by expectations on the commands called by the methods.
This moves some of the code that is specific to remote queries out of
the `run-remote-query.ts` file and instead places it in separate files
that only deal with remote queries, rather than also dealing with
variant analyses.
Base automatically changed from koesie10/reduce-nesting-remote-query to main November 11, 2022 13:46
@koesie10 koesie10 merged commit 0d15768 into main Nov 11, 2022
@koesie10 koesie10 deleted the koesie10/split-remote-query-flows branch November 11, 2022 14:15
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