Skip to content

Move LocalQueries commands to be member functions#2245

Merged
dbartol merged 2 commits intomainfrom
dbartol/local-queries-commands
Mar 31, 2023
Merged

Move LocalQueries commands to be member functions#2245
dbartol merged 2 commits intomainfrom
dbartol/local-queries-commands

Conversation

@dbartol
Copy link
Copy Markdown
Contributor

@dbartol dbartol commented Mar 28, 2023

This PR implements a review suggestion from #2244. I've made it a separate, dependent PR to avoid making the original PR's diffs even worse.

@dbartol dbartol requested a review from a team as a code owner March 28, 2023 16:50
@dbartol dbartol requested a review from koesie10 March 28, 2023 22:23
@dbartol dbartol added code cleanup Not directly user-visible code improvements Complexity: Low A good task for newcomers to learn, or experienced team members to complete quickly. labels Mar 28, 2023
@aeisenberg
Copy link
Copy Markdown
Contributor

Just wanting to be clear. [dbartol/debug-adapter](https://github.com/github/vscode-codeql/tree/dbartol/debug-adapter) has no PR associated with it. Is this PR targeting the right branch?

const astBuilder = createAstBuilder();
const roots = await astBuilder.getRoots();

const bqrsPath = path.normalize("/a/b/c/results.bqrs");
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.

Any understanding of why this test had to be changed?

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.

I think that change went into the base PR, but the reason for the change was that previously, the test code passed in "/a/b/c" as the path to the BQRS file itself, which got passed through the implementation verbatim and came out the other end with forward slashes intact. With my changes, "/a/b/c" is treated as the query output directory, which goes through some path calls (join, etc.) before the BQRS path comes out the other end with platform-specific separators.

@dbartol dbartol changed the base branch from dbartol/debug-adapter to dbartol/debug-context March 29, 2023 18:16
@dbartol
Copy link
Copy Markdown
Contributor Author

dbartol commented Mar 29, 2023

Just wanting to be clear. [dbartol/debug-adapter](https://github.com/github/vscode-codeql/tree/dbartol/debug-adapter) has no PR associated with it. Is this PR targeting the right branch?

Oh, yikes! That's the wrong target branch. I'll fix this in a few minutes. Fixed.

@dbartol dbartol force-pushed the dbartol/local-queries-commands branch from 9b636ec to aef836f Compare March 29, 2023 18:21
Base automatically changed from dbartol/debug-context to main March 30, 2023 14:23
@dbartol dbartol merged commit b67e3b7 into main Mar 31, 2023
@dbartol dbartol deleted the dbartol/local-queries-commands branch March 31, 2023 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code cleanup Not directly user-visible code improvements Complexity: Low A good task for newcomers to learn, or experienced team members to complete quickly.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants