-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Dont sent telemetry in forks #21377
Dont sent telemetry in forks #21377
Conversation
…om/paulacamargo25/vscode-python into 20941-Dont-sent-telemetry-in-forks
…om/paulacamargo25/vscode-python into 20941-Dont-sent-telemetry-in-forks
* @returns {boolean} | ||
*/ | ||
export function isTelemetryDisabled(workspaceService: IWorkspaceService): boolean { | ||
const settings = workspaceService.getConfiguration('telemetry').inspect<boolean>('enableTelemetry')!; |
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.
Are we removing this setting?
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 checked the code and we don't use that function anywhere.
export function isTelemetryDisabled(): boolean { | ||
const packageJsonPath = path.join(EXTENSION_ROOT_DIR, 'package.json'); | ||
const packageJson = fs.readJSONSync(packageJsonPath); | ||
return !packageJson.enableTelemetry; |
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.
File I/O is expensive for this check, this is being done every time a telemetry is sent. See #22991.
Can we do this using setting instead of package.json
? @paulacamargo25 @karthiknadig
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.
you should have access to package json as a part of the extension API.
/**
* The parsed contents of the extension's package.json.
*/
readonly packageJSON: any;
For #22991 Introduced by #21377 cc/ @DonJayamanne
Closed #20941