-
Notifications
You must be signed in to change notification settings - Fork 370
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
Add index list command #537
Add index list command #537
Conversation
Needs cli integration tests. :) |
cmd/krew/cmd/index.go
Outdated
// indexCmd represents the index command | ||
var indexCmd = &cobra.Command{ | ||
Use: "index", | ||
Short: "Perform krew index commands", |
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 doesn't look good in the help message:
Available Commands:
index Perform krew index commands
let's say something more meaningful.
cmd/krew/cmd/index.go
Outdated
dirs, err := ioutil.ReadDir(paths.IndexBase()) | ||
if err != nil { | ||
return errors.Wrapf(err, "failed to read directory %s", paths.IndexBase()) | ||
} | ||
var rows [][]string | ||
for _, dir := range dirs { | ||
indexName := dir.Name() | ||
remote, err := gitutil.GetRemoteURL(paths.IndexPath(indexName)) | ||
if err != nil { | ||
return errors.Wrapf(err, "failed to list the remote URL for index %s", indexName) | ||
} |
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.
Soon you'll find out you need a package where this lives and is tested.
I propose a package that does
ListIndexes(path string) ([]Index, error)
RemoveIndex(path, name string) error
AddIndex...
We can start small, so let's create a pkg, and let's add tests.
Right now the code I just highlighted here has 0 tests.
cmd/krew/cmd/index.go
Outdated
Short: "Perform krew index commands", | ||
Long: "Perform krew index commands such as adding and removing indexes.", | ||
Args: cobra.NoArgs, | ||
Hidden: true, // remove this once multi-index is enabled |
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.
use // TODO
syntax
internal/gitutil/git.go
Outdated
@@ -72,10 +72,11 @@ func EnsureUpdated(uri, destinationPath string) error { | |||
|
|||
// GetRemoteURL returns the url of the remote origin | |||
func GetRemoteURL(dir string) (string, error) { | |||
return exec(dir, "config", "--get", "remote.origin.url") | |||
remote, err := Exec(dir, "config", "--get", "remote.origin.url") | |||
return strings.TrimRight(remote, "\n\r"), err |
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.
You can use strings.TrimFunc(unicode.IsSpace)
Furthermore, maybe just move this to Exec, as other cmds are likely going to need 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.
Ended up using strings.TrimSpace
, the linter complained about strings.TrimFunc('...', unicode.IsSpace)
cmd/krew/cmd/index.go
Outdated
"sigs.k8s.io/krew/pkg/constants" | ||
) | ||
|
||
// indexCmd represents the index command | ||
var indexCmd = &cobra.Command{ | ||
Use: "index", | ||
Short: "Perform krew index commands", | ||
Long: "Perform krew index commands such as adding and removing indexes.", | ||
Short: "Add, remove, and list indexes", |
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.
Let’s just say:
Manage custom plugin indexes
& long
Manage which repositories are used to discover and install plugins from.
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahmetb, chriskim06 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/lgtm |
Initial implementation of
kubectl krew index list
. This prints the indexes in a table in the same format askubectl krew list
:I slightly refactored the
exec
function in gitutil to return the output of the command. The existing uses of it don't need it but I'm using the output fromgit config --get remote.origin.url
to get the index's remote url. Let me know if there's a better way to get the url from git, that was the only way I knew how.Related issue: #483