-
Notifications
You must be signed in to change notification settings - Fork 6.5k
feat(cli): add argocd context subcommands
#25077
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: master
Are you sure you want to change the base?
feat(cli): add argocd context subcommands
#25077
Conversation
Signed-off-by: Omar Nasser <[email protected]>
Signed-off-by: Omar Nasser <[email protected]>
Signed-off-by: Omar Nasser <[email protected]>
🔴 Preview Environment stopped on BunnyshellSee: Environment Details | Pipeline Logs Available commands (reply to this comment):
|
|
/component cli |
Signed-off-by: Omar Nasser <[email protected]>
Signed-off-by: Omar Nasser <[email protected]>
Signed-off-by: Omar Nasser <[email protected]>
| # Switch Argo CD context | ||
| argocd context cd.argoproj.io | ||
| argocd context switch cd.argoproj.io |
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.
how about argocd context 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.
If I'm not wrong, the switch command does exactly the same as use command, do you mean to change the name of the command from switch to be use?
If this is not what you mean, can you tell me the difference between them?
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.
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.
@ppapapetrou76 I added use as an alias for switch command in the latest commit.
Do you think I should replace switch completely with use? or keep them together with alias?
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 think argocd context use cd.argoproj.io would be a more intuitive and consistent choice here.
The word “use” aligns with common CLI patterns like kubectl config use-context and clearly conveys the action of setting the active context.
You could still keep switch as an alias, but use would be better as the primary example.
| err := deleteContext(args[0], clientOpts.ConfigPath) | ||
| errors.CheckError(err) | ||
| return | ||
| func NewContextListCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command { |
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.
| func NewContextListCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command { | |
| // NewContextListCommand lists the contexts | |
| func NewContextListCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command { |
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.
Do you think we should to stick to the function comments format in app and login (i.e. https://github.com/argoproj/argo-cd/blob/master/cmd/argocd/commands/app.go#L124)
or apply your change and make the comment more meaningful (Anyway I see that the usage of that commands make it clear anyway) but I'd wait for your answer
nitishfy
left a comment
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.
Thank you for the PR. The code looks good to me. Here are few suggestions:
- Please write comments in the code wherever required, eg. in the code for switch command.
- Write unit tests for the utility functions.
- You'll be required to run codegen to update the docs too.
argocd context subcommands
util/localconfig/localconfig.go
Outdated
| return false | ||
| } | ||
|
|
||
| func (l *LocalConfig) GetContext(ctxName string) (*ContextRef, error) { |
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.
please write unit tests.
Co-authored-by: Nitish Kumar <[email protected]> Signed-off-by: Omar Nasser <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #25077 +/- ##
==========================================
+ Coverage 60.83% 62.28% +1.44%
==========================================
Files 350 351 +1
Lines 60430 49273 -11157
==========================================
- Hits 36762 30688 -6074
+ Misses 20754 15663 -5091
- Partials 2914 2922 +8 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Omar Nasser <[email protected]>
3580699 to
9c1e965
Compare
Signed-off-by: Omar Nasser <[email protected]>
Signed-off-by: Omar Nasser <[email protected]> Provide use command as alias for switch Signed-off-by: Omar Nasser <[email protected]>
Signed-off-by: Omar Nasser <[email protected]>
| func NewContextSwitchCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command { | ||
| command := &cobra.Command{ | ||
| Use: "switch", | ||
| Aliases: []string{"use"}, | ||
| Short: "Switch Argo CD Context", | ||
| Example: ` # Switch Argo CD Context | ||
| argocd context switch cd.argoproj.io`, |
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.
| func NewContextSwitchCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command { | |
| command := &cobra.Command{ | |
| Use: "switch", | |
| Aliases: []string{"use"}, | |
| Short: "Switch Argo CD Context", | |
| Example: ` # Switch Argo CD Context | |
| argocd context switch cd.argoproj.io`, | |
| func NewContextUseCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command { | |
| command := &cobra.Command{ | |
| Use: "use", | |
| Aliases: []string{"switch"}, | |
| Short: "Set Argo CD Context", | |
| Example: ` # Set Argo CD context | |
| argocd context use cd.argoproj.io`, |
Mangaal
left a comment
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 have added a few comments and suggestions, feel free to resolve them if you agree with the changes.
Related #19867