Add telemetry enable/disable command #2
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR introduces a new 'telemetry' command to the Vercel CLI, allowing users to manage telemetry collection settings.
- Added
packages/cli/src/commands/telemetry/directory with new files for enabling, disabling, and checking telemetry status - Modified
packages/cli/src/index.tsto include the new telemetry command and adjust initialization order - Updated
internals/types/index.d.tsto replacecollectMetricswith a more granulartelemetryproperty inGlobalConfig - Implemented unit tests in
packages/cli/test/unit/commands/telemetry/index.test.tsto ensure proper functionality of the new command
11 file(s) reviewed, 11 comment(s)
Edit PR Review Bot Settings
| @@ -40,5 +40,6 @@ export default new Map([ | |||
| ['targets', 'target'], | |||
| ['team', 'teams'], | |||
| ['teams', 'teams'], | |||
| ['telemetry', 'telemetry'], | |||
There was a problem hiding this comment.
style: Consider adding a comment explaining the purpose of the telemetry command for better code documentation
| description: 'Shows whether telemetry collection is enabled or disabled', | ||
| arguments: [], | ||
| options: [], | ||
| examples: [], |
There was a problem hiding this comment.
style: Add an example for the 'status' subcommand to improve user guidance
| description: 'Enables telemetry collection', | ||
| arguments: [], | ||
| options: [], | ||
| examples: [], |
There was a problem hiding this comment.
style: Add an example for the 'enable' subcommand to improve user guidance
| description: 'Disables telemetry collection', | ||
| arguments: [], | ||
| options: [], | ||
| examples: [], |
There was a problem hiding this comment.
style: Add an example for the 'disable' subcommand to improve user guidance
| if (parsedArguments.flags['--help']) { | ||
| client.output.print( | ||
| help(telemetryCommand, { columns: client.stderr.columns }) | ||
| ); | ||
| } |
There was a problem hiding this comment.
logic: The function returns undefined after printing help. Consider returning 0 to indicate successful execution.
| const { subcommand } = getSubcommand( | ||
| parsedArguments.args.slice(1), | ||
| COMMAND_CONFIG | ||
| ); |
There was a problem hiding this comment.
style: Consider using parsedArguments.args[1] instead of slicing, as you're only interested in the second argument.
| const status: 'disabled' | 'enabled' = | ||
| client.config.telemetry?.enabled === false ? 'disabled' : 'enabled'; |
There was a problem hiding this comment.
style: Consider using a boolean type for status instead of string literals
| @@ -134,12 +141,19 @@ export class TelemetryEventStore { | |||
| this.events = []; | |||
| } | |||
|
|
|||
| enabled() { | |||
| return this.config?.enabled === false ? false : true; | |||
There was a problem hiding this comment.
style: Consider simplifying this to return this.config?.enabled !== false; for better readability
| if (this.enabled()) { | ||
| // send events to the server | ||
| } |
There was a problem hiding this comment.
logic: Implement the logic to send events to the server
| constructor(opts: { | ||
| output: Output; | ||
| isDebug?: boolean; | ||
| config: GlobalConfig['telemetry']; | ||
| }) { |
There was a problem hiding this comment.
style: Consider making config a required parameter to ensure it's always provided
No description provided.