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

#23671 Added synopsis for operator root and operator gossip command. #23855

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

raj-hegde
Copy link

@raj-hegde raj-hegde commented Aug 22, 2024

Synopsis comment for both gossip and root section has been added. Added the respective command files for each of the subcommands.

closes #14881
closes #23671

Copy link

hashicorp-cla-app bot commented Aug 22, 2024

CLA assistant check
All committers have signed the CLA.

@raj-hegde
Copy link
Author

Please give feedback if I did anything wrong.

Copy link
Member

@jrasell jrasell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @raj-hegde and thank you very much for raising this PR. I have added some minor suggestions for you to look at. Once these are resolved, I will get this PR merged.

Also, if you could add a changelog entry via the make cl command, that would be awesome.

Comment on lines 15 to 18
Usage: nomad operator gossip [options]

This command is accessed by using one of the subcommands below.
`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When running the command locally, the output format seemed slightly off. The whitespace suggestion fixes that and the change to the usage keeps more consistency with other commands and correctly shows subcommand expectation.

Suggested change
Usage: nomad operator gossip [options]
This command is accessed by using one of the subcommands below.
`
Usage: nomad operator gossip <subcommand> [options] [args]
This command is accessed by using one of the subcommands below.
`


func (f *OperatorGossipCommand) Name() string { return "operator gossip" }

func (f *OperatorGossipCommand) Run(args []string) int {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The args function parameter is unused, so we can use an underscore to show this is ignored.

Suggested change
func (f *OperatorGossipCommand) Run(args []string) int {
func (f *OperatorGossipCommand) Run(_ []string) int {

Comment on lines 15 to 18
Usage: nomad operator root [options]

This command is accessed by using one of the subcommands below.
`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When running the command locally, the output format seemed slightly off. The whitespace suggestion fixes that and the change to the usage keeps more consistency with other commands and correctly shows subcommand expectation.

Suggested change
Usage: nomad operator root [options]
This command is accessed by using one of the subcommands below.
`
Usage: nomad operator root <subcommand> [options] [args]
This command is accessed by using one of the subcommands below.
`


func (f *OperatorRootCommand) Name() string { return "operator gossip" }

func (f *OperatorRootCommand) Run(args []string) int {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The args function parameter is unused, so we can use an underscore to show this is ignored.

Suggested change
func (f *OperatorRootCommand) Run(args []string) int {
func (f *OperatorRootCommand) Run(_ []string) int {

return "Provides access to root encryption keys"
}

func (f *OperatorRootCommand) Name() string { return "operator gossip" }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this may have been a copy/paste error :D

Suggested change
func (f *OperatorRootCommand) Name() string { return "operator gossip" }
func (f *OperatorRootCommand) Name() string { return "operator root" }

@jrasell jrasell self-assigned this Sep 18, 2024
@jrasell jrasell added backport/ent/1.6.x+ent Changes are backported to 1.6.x+ent backport/ent/1.7.x+ent Changes are backported to 1.7.x+ent backport/ent/1.8.x+ent Changes are backported to 1.8.x+ent backport/1.8.x backport to 1.8.x release line labels Sep 18, 2024
@raj-hegde
Copy link
Author

Hi @jrasell, Thanks for the feedback I'll work on it right away.

@raj-hegde
Copy link
Author

Hi @jrasell removed white spaces keeping consistency with other commands and changed the args function operator to underscore. Also made a changlelog entry.

Any feedback is greatly appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/ent/1.6.x+ent Changes are backported to 1.6.x+ent backport/ent/1.7.x+ent Changes are backported to 1.7.x+ent backport/ent/1.8.x+ent Changes are backported to 1.8.x+ent backport/1.8.x backport to 1.8.x release line
Projects
2 participants