Skip to content
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

Show what's updated in "update" command (or its invocations indirectly) #443

Closed
ahmetb opened this issue Dec 24, 2019 · 2 comments · Fixed by #457
Closed

Show what's updated in "update" command (or its invocations indirectly) #443

ahmetb opened this issue Dec 24, 2019 · 2 comments · Fixed by #457
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. priority/backlog Higher priority than priority/awaiting-more-evidence. priority/P3 P3 issues or PRs

Comments

@ahmetb
Copy link
Member

ahmetb commented Dec 24, 2019

This is something Homebrew has, and I've been looking to have it in Krew, too.

When you run krew update (or indirectly, through krew install or krew upgrade), you'll see a message: Updated the local copy of plugin index. This message is nice, but it doesn't provide the big picture.

I think for update command specifically, it would be good if we can show something like:

$ kubectl krew install foo
\
 │ Updating the local copy of plugin index.
 │   New plugins available:
 │     * multiwatch v0.1.0
 │     * view-secret v0.1.9
 │   The following plugins have new version:
 │     * whoami v0.3.2 -> v0.4.0
 │     * rbac-lookup v1.0.2 -> v1.0.3
 │     * get-all v0.8.0 -> v0.9.1  🚨 (a sign to indicate it's installed)
/
Installing plugin "foo".
[...]

Some things to consider:

  • requires significant change to underlying update mechanism, as we need to parse all index, store in-memory, then compare (worth it?)

  • the above example could be too much output for the install command, maybe we figure out a one-liner version like:

     New plugins available: multiwatch v0.1.0, multiwatch v0.1.0
     Upgrades available: get-all v0.8.0 -> v0.9.1 <--- 🚨only installed ones
    
  • this should probably be tested only as unit tests as it would be unnecessarily complex to make integration tests for the update scenarios

I'm supportive of this change only if it can be done with small effort.

/priority P3
/priority backlog
/good-first-issue

@k8s-ci-robot

This comment has been minimized.

@k8s-ci-robot k8s-ci-robot added priority/P3 P3 issues or PRs priority/backlog Higher priority than priority/awaiting-more-evidence. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. labels Dec 24, 2019
@artmello
Copy link
Contributor

artmello commented Jan 4, 2020

Linked PR implements a prototype for this feature. Above some example outputs of it so suggestions can be made about how to improve it.

When nothing was changed on index

./out/bin/krew-darwin_amd64 update
Updated the local copy of plugin index.

When system has following Name/Version information on index:

"foo":  "v0.1.0",
"bar":  "v0.1.0",
"fizz": "v0.1.0",
"buzz": "v0.1.0",

and receive following information after update:

"new-foo": "v0.1.0",
"new-bar": "v0.1.0",
"foo":     "v0.1.0",
"bar":     "v0.1.0",
"fizz":    "v1.1.0",
"buzz":    "v1.1.0",

With fizz plugin being already installed on the system

./out/bin/krew-darwin_amd64 update
Updated the local copy of plugin index.
  New plugins available:
    * new-bar v0.1.0
    * new-foo v0.1.0
  The following plugins have new version:
    * fizz v0.1.0 -> v1.1.0 (!)
    * buzz v0.1.0 -> v1.1.0

Both lists are sorted by Name (with installed plugins showing up first for the new versions list. By now this is the same output for all commands that call update. If we agree this is the desired approach, I plan to work on a simplified version of such output for command that call update as a pre requisite (i.e install).

PR is missing unit tests and failing on integration tests right now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. priority/backlog Higher priority than priority/awaiting-more-evidence. priority/P3 P3 issues or PRs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants