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

cmd: change "node list" command for "list node" #541

Merged
merged 1 commit into from
May 1, 2021

Conversation

rolinh
Copy link
Member

@rolinh rolinh commented Apr 30, 2021

We recently decided to try and uniformize the Hubble CLI subcommands and
agreed on a [verb] [noun] approach. Given that there hasn't been any
release of Hubble CLI with the nodes subcommand yet, change the
hubble nodes list command for hubble list nodes.

We recently decided to try and uniformize the Hubble CLI subcommands and
agreed on a `[verb] [noun]` approach. Given that there haven't been any
release of Hubble CLI with the `nodes` subcommand yet, change the
`hubble nodes list` command for `hubble list nodes`.

Signed-off-by: Robin Hahling <robin.hahling@gw-computing.net>
@rolinh rolinh added the release-note/misc This PR makes changes that have no direct user impact. label Apr 30, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 1, 2021
@michi-covalent michi-covalent merged commit 64ce314 into master May 1, 2021
@michi-covalent michi-covalent deleted the pr/rolinh/list-cmd branch May 1, 2021 06:12
@kaworu
Copy link
Member

kaworu commented May 3, 2021

One thing to consider is that we don't mirror cilium node list after this patch.

@gandro
Copy link
Member

gandro commented May 3, 2021

One thing to consider is that we don't mirror cilium node list after this patch.

FWIW, I think that's fine. Cilium's CLI uses noun verb almost everywhere, whereas Hubble is doubling down on verb noun.

@rolinh
Copy link
Member Author

rolinh commented May 3, 2021

This raises a point about uniformity with the new cilium cli. It seems to be mixed currently (cilium install but cilium hubble enable). However, I think it's better to discuss this in our next weekly meeting or in Slack.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants