-
Notifications
You must be signed in to change notification settings - Fork 218
Deprecate codeQL.runningQueries.saveCache setting
#4210
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
Conversation
1f960f1 to
74ac0fb
Compare
extensions/ql-vscode/CHANGELOG.md
Outdated
| ## [UNRELEASED] | ||
|
|
||
| - Rename command "CodeQL: Trim Overlay Base Cache" to "CodeQL: Trim Cache to Overlay-Base" for consistency with "CodeQL: Warm Overlay-Base Cache for [...]" commands. [#4204](https://github.com/github/vscode-codeql/pull/4204) | ||
| - Deprecate the setting (`codeQL.runningQueries.saveCache`) to aggressively save intermediate results to the disk cache. [#4210](https://github.com/github/vscode-codeql/pull/4210) |
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.
(nit) There is some ambiguity, as the sentence can be parsed in two ways:
- Deprecate (the setting to aggressively save ...)
- (Deprecate the setting) to aggressively save ...
Please consider rewording it for greater clarity. Thanks!
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.
Good point; fixed.
74ac0fb to
f7fcc9e
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.
Pull request overview
This PR deprecates the codeQL.runningQueries.saveCache setting which aggressively saved intermediate query results to disk cache but generally degraded performance. The setting is marked as deprecated in VS Code's configuration UI (showing for users who previously set it) while removing its functionality from the codebase.
Key changes:
- Added deprecation message to the setting in package.json
- Removed all code that implemented the saveCache functionality
- Updated tests to remove references to the deprecated setting
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| extensions/ql-vscode/package.json | Added deprecationMessage to the saveCache setting to mark it as deprecated in VS Code UI |
| extensions/ql-vscode/src/config.ts | Removed SAVE_CACHE_SETTING constant, saveCache property from QueryServerConfig interface, and saveCache getter from QueryServerConfigListener class |
| extensions/ql-vscode/src/query-server/query-server-client.ts | Removed the code that added the --save-cache flag to query server arguments |
| extensions/ql-vscode/test/vscode-tests/minimal-workspace/config.test.ts | Removed saveCache from the test configuration settings array |
| extensions/ql-vscode/test/vscode-tests/cli-integration/query-server/query-server-client.test.ts | Removed saveCache: false from the QueryServerClient test initialization |
| extensions/ql-vscode/CHANGELOG.md | Added entry documenting the deprecation of the setting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This setting was a bit of a footgun for users, and generally only caused performance to get worse.
By adding a
deprecationMessagetopackage.json, the docs suggest, and my testing confirms, that this will cause the setting to be hidden for users who haven't set it. For those who have explicitly set it (to either true or false), the settings UI will show the deprecation message.