-
Notifications
You must be signed in to change notification settings - Fork 3.7k
fix: sync telemetry enabled button with ide settings #8497
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
base: main
Are you sure you want to change the base?
Conversation
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.
1 issue found across 10 files
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="extensions/vscode/src/VsCodeIde.ts">
<violation number="1" location="extensions/vscode/src/VsCodeIde.ts:703">
Await the VS Code configuration update so the promise resolves only after the telemetry setting is written.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
extensions/vscode/src/VsCodeIde.ts
Outdated
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.
Await the VS Code configuration update so the promise resolves only after the telemetry setting is written.
Prompt for AI agents
Address the following comment on extensions/vscode/src/VsCodeIde.ts at line 703:
<comment>Await the VS Code configuration update so the promise resolves only after the telemetry setting is written.</comment>
<file context>
@@ -698,6 +698,12 @@ class VsCodeIde implements IDE {
}
+
+ async updateTelemetryEnabled(enabled: boolean): Promise<void> {
+ vscode.workspace
+ .getConfiguration("continue")
+ .update("telemetryEnabled", enabled, vscode.ConfigurationTarget.Global);
</file context>
| vscode.workspace | |
| await vscode.workspace |
✅ Addressed in d7c5494
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.
@uinstinct when ide settings are changed it comes through as an ide settings update which causes a config reload, which accounts for the ide setting when setting config.allowAnonymousTelemetry. It should already sync properly, although it might result in the toggle being un-toggleable if set to false in IDE settings (probably fine)
Could you explain the background a bit more?
Right. The scenario mentioned by you happens for syncing vscode settings page --> gui settings page configuration. The current PR adds the ability to for syncing gui settings page --> vscode settings page which was absent. |
|
I think we've more or less deprecated the allowAnonymousTelemetry IDE setting or at least it's not the preferred way to configure. The issue with this approach is that it prevents people from just editing the JSON because now it's out of sync. E.g. the only way to change allowAnonymousTelemetry is now through the toggle, which would create a security concern affecting the preferred config approach (globalContext) Potentially a better UI would be to disable the toggle if set by the UI? |
True. We should have one setting and that should be the toggle in extension gui settings page. |
|
@uinstinct the current functionality is that if allowAnonymousTelemetry is detected in config.json it migrates the value to shared config and then removes it from config.json. This can stay the same. |
|
@RomneyDa Now I have removed the isTelemetryEnabled setting from vscode and jetbrains. Hence currently the only way to set allowAnonymousTelemetry is now through the settings page toggle, config.json and org policy |
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.
@uinstinct we should probably keep support for the IDE setting since if people have it set to false we will then be violating their prior setting
Description
The gui settings for telemetry enabling was in sync with the vscode ide settings only when the ide restarted. This PR fixes that.
Resolves CON-4319
AI Code Review
@continue-reviewChecklist
Screen recording or screenshot
before.mp4
after.mp4
Tests
[ What tests were added or updated to ensure the changes work as expected? ]
Summary by cubic
Decouples telemetry from IDE settings and makes the in-app toggle the single source of truth. Removes the VS Code continue.telemetryEnabled setting and the isTelemetryEnabled protocol across core, VS Code, and JetBrains (CON-4319).
Written for commit 9a0265c. Summary will update automatically on new commits.