-
Notifications
You must be signed in to change notification settings - Fork 191
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Spurious Compilation Errors #3845
Conversation
...ions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts
Fixed
Show fixed
Hide fixed
...ions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts
Fixed
Show fixed
Hide fixed
656363b
to
a97fbd6
Compare
extensions/ql-vscode/test/vscode-tests/minimal-workspace/qlpack-generator.test.ts
Fixed
Show fixed
Hide fixed
a97fbd6
to
9c87113
Compare
Koen pointed out this could be generalized: https://github.com/github/codeql-core/issues/4654#issuecomment-2506073240 -> draft mode |
9c87113
to
788bb4c
Compare
788bb4c
to
6fd3d20
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left a few small comments, but I'll leave a full review of this approach to someone that is more familiar with the language server/CLI interactions.
Thank you for your comments <3 I'll work on them next week. |
Thanks @koesie10 and @aeisenberg for your reviews! They've increased my confidence, as well as made the PR shorter and simpler :) This commit is the result: 17542d1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, LGTM! I've also tested this using the reproduction steps in the public and internal issues and it doesn't show any compilation errors once the pack has been installed.
Can you please add "Closes #3220" to the issue description since this PR fixes that issue?
Thanks for taking the time to do that, I appreciate it!
done! |
Closes #3220
issue: https://github.com/github/codeql-core/issues/4654
This notifies the language server of newly downloaded packs once a pack is added to the current ql pack, or a pack's dependencies are installed.
This is the user facing behaviour I observed:
Dear Reviewer
~/.codeql/packages
subdirectories, ...). Chances are, there's room for improvement.