-
Notifications
You must be signed in to change notification settings - Fork 77
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
feat(cli): Token Support #3255
feat(cli): Token Support #3255
Conversation
cli/cmd/start_cmd.go
Outdated
@@ -49,6 +52,7 @@ func init() { | |||
startCmd.Flags().StringVarP(&saveParams.organizationID, "organization", "", "", "organization id") | |||
startCmd.Flags().StringVarP(&saveParams.environmentID, "environment", "", "", "environment id") | |||
startCmd.Flags().StringVarP(&saveParams.agentApiKey, "api-key", "", "", "agent api key") | |||
startCmd.Flags().StringVarP(&saveParams.tokenApiKey, "token", "", defaultToken, "token api key") |
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.
cli-api-key
instead of token
?
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'm kind of late to this discussion. But I think that cli-api-key
is too close to api-key
and it's hard to understand what's the difference between them.
I would suggest using either token
or a more meaningful name for this flag (which I have no suggestions yet). The idea behind using token
is that the user will get the value from the Token
tab, so will be more intuitive for the user to create a connection between the value and the flag they need to use.
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 agree with this statement too, I think we can go with token as it matches the SaaS cloud, then in the future we can decide to change it
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.
Left a comment on the flag name discussion. It's not a blocker
This PR adds token support for the
tracetest start
commandChanges
Checklist
Loom video
https://www.loom.com/share/a1405279a47a429cae4fe0bb8df25233