-
Notifications
You must be signed in to change notification settings - Fork 7k
feat: add analytics config setting #8350
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
7602905 to
bc086d6
Compare
|
@codex review |
bc086d6 to
c35195c
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// When `false`, disables analytics across Codex product surfaces in this machine. | ||
| /// Defaults to `true`. | ||
| pub analytics: bool, |
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.
Wire analytics flag into runtime behavior
The new Config::analytics field is never read anywhere in the codebase (a repo-wide search for analytics only finds these config definitions and docs), so setting analytics = false will not actually disable any analytics. That makes the documented setting a no-op and risks misleading users who expect telemetry to be turned off. Consider plumbing this flag into the analytics/telemetry initialization paths or removing the setting until it is enforced.
Useful? React with 👍 / 👎.
|
Could we make this a bit more complex? I expect we will have multiple fields in there in the end so could we have something like: And btw we are not allowed to enable it by default in some juridiction so we must check with legal (I already sent the request but no answer yet) |
No description provided.