-
Notifications
You must be signed in to change notification settings - Fork 43
🌱 change all 'operator(s)' references to 'extension(s)' in olmv1 commands #227
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
package olmv1 | ||
|
||
import ( | ||
"github.com/spf13/cobra" | ||
|
||
"github.com/operator-framework/kubectl-operator/internal/cmd/internal/log" | ||
v1action "github.com/operator-framework/kubectl-operator/internal/pkg/v1/action" | ||
"github.com/operator-framework/kubectl-operator/pkg/action" | ||
) | ||
|
||
// NewExtensionInstalledGetCmd handles get commands in the form of: | ||
// extension(s) [extension_name] - this will either list all the installed extensions | ||
// if no extension_name has been provided or display the details of the specific | ||
// one otherwise | ||
func NewExtensionInstalledGetCmd(cfg *action.Configuration) *cobra.Command { | ||
i := v1action.NewExtensionInstalledGet(cfg) | ||
i.Logf = log.Printf | ||
|
||
cmd := &cobra.Command{ | ||
Use: "extension [extension_name]", | ||
Aliases: []string{"extensions [extension_name]"}, | ||
azych marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Args: cobra.RangeArgs(0, 1), | ||
Short: "Display one or many installed extensions", | ||
Run: func(cmd *cobra.Command, args []string) { | ||
if len(args) == 1 { | ||
i.ExtensionName = args[0] | ||
} | ||
installedExtensions, err := i.Run(cmd.Context()) | ||
if err != nil { | ||
log.Fatalf("failed getting installed extension(s): %v", err) | ||
} | ||
|
||
printFormattedExtensions(installedExtensions...) | ||
}, | ||
} | ||
|
||
return cmd | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,26 +8,26 @@ import ( | |
"github.com/operator-framework/kubectl-operator/pkg/action" | ||
) | ||
|
||
func NewOperatorUninstallCmd(cfg *action.Configuration) *cobra.Command { | ||
u := v1action.NewOperatorUninstall(cfg) | ||
func NewExtensionUninstallCmd(cfg *action.Configuration) *cobra.Command { | ||
u := v1action.NewExtensionUninstall(cfg) | ||
u.Logf = log.Printf | ||
|
||
cmd := &cobra.Command{ | ||
Use: "uninstall <operator>", | ||
Short: "Uninstall an operator", | ||
Long: `Uninstall deletes the named Operator object. | ||
Use: "uninstall <extension>", | ||
Short: "Uninstall an extension", | ||
Long: `Uninstall deletes the named extension object. | ||
|
||
Warning: this command permanently deletes objects from the cluster. If the | ||
uninstalled Operator bundle contains CRDs, the CRDs will be deleted, which | ||
uninstalled extension bundle contains CRDs, the CRDs will be deleted, which | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should not use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is no functional change, it's just one-word rename, so I think bundle(s) are still being used to install operators/extensions. That said, this is a simple rename PR and @LalatenduMohanty is working on install/uninstall commands that this comment touches. How about we have those discussions on that incoming PR where they belong? |
||
cascades to the deletion of all operands. | ||
`, | ||
Args: cobra.ExactArgs(1), | ||
Run: func(cmd *cobra.Command, args []string) { | ||
u.Package = args[0] | ||
if err := u.Run(cmd.Context()); err != nil { | ||
log.Fatalf("uninstall operator: %v", err) | ||
log.Fatalf("uninstall extension: %v", err) | ||
} | ||
log.Printf("deleted operator %q", u.Package) | ||
log.Printf("deleted extension %q", u.Package) | ||
}, | ||
} | ||
return cmd | ||
|
This file was deleted.
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.
I'm not sure about replacing extension with package name here. I think commonly and for user-facing purposes you'd say you want to install an (cluster)extension?
Package name seems like an implementation detail at this point and at least to me it could confuse people because we'd have crud operations mentioning (cluster)extension and then a package terminology at the root level.
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 Ankita. Here is my suggestion
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.
this is a simple rename PR and @LalatenduMohanty is working on install/uninstall commands that this comment touches. How about we have those discussions on that incoming PR where they belong?
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 fine with redoing the install help text in the v1 install/uninstall commands PR.
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.
Thats fine. I will fix it in my PR.