Skip to content

✨ Add curated list of subcommands to core Short - #1952

Closed
shrink wants to merge 4 commits into
cli:trunkfrom
shrink:add-curated-list-short
Closed

shrink wants to merge 4 commits into
cli:trunkfrom
shrink:add-curated-list-short

Conversation

@shrink

@shrink shrink commented Sep 24, 2020

Copy link
Copy Markdown

Adds cmdutil.ApplyCuratedShort which accepts a command, a resource name (e.g: Pull Requests) and a list of subcommand names which are then used to generate a human friendly list of subcommands supported for the Short description of the command.

Note: this is a speculative example to be reviewed by @ampinsk -- in response to their comment on #1777. The PR is ready to go but marked as Draft so it isn't merged before @ampinsk has decided if this is the right direction :-)

CORE COMMANDS
  gist:       create, view and edit Gists (+1 more)
  issue:      create, view and close Issues (+3 more)
  pr:         create, review and merge Pull Requests (+9 more)
  release:    create, view and download Releases (+3 more)
  repo:       create, fork and garden Repositories (+3 more)

ApplyCuratedShort is explicitly added to a command after the subcommands have been registered, e.g:

//                              Resource             Curated list of commands
cmdutil.ApplyCuratedShort(cmd, "Releases", []string{"create", "view", "download"})

Some additional examples (as captured in the test cases):

1 command, all curated:

  pr:         list Pull Requests

2 commands, all curated:

  pr:         list and view Pull Requests

3 commands, all curated:

  pr:         list, view and create Pull Requests

4 commands, 3 curated:

  pr:         list, view and create Pull Requests (+1 more)

5 commands, 3 curated:

  pr:         list, view and create Pull Requests (+2 more)

2 commands, 2 curated, List capitalised:

  pr:         List and view Pull Requests

I added the curated list to each of the core commands, choosing the subcommands that best represent the breadth of functionality supported for that resource.

Also, if this PR does make it into consideration, worth noting that this is my first real world contribution in Go so I'd recommend a close review 🔎

Thanks!

Add cmdutil.ApplyCuratedShort which accepts a command, a resource name (e.g: `Pull Requests`) and a list of subcommand names which are then used to generate a human friendly list of subcommands supported for the `Short` description of the command.

Closes #1777
@Okeakwalam27

This comment has been minimized.

@samcoe samcoe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for taking on this work 🙇

Code wise this looks really good to me. I like the thorough tests!

@vilmibm vilmibm removed the community label Sep 29, 2020
@vilmibm
vilmibm requested a review from ampinsk September 29, 2020 19:14
@vilmibm vilmibm self-assigned this Jan 19, 2021
@vilmibm

vilmibm commented Jan 19, 2021

Copy link
Copy Markdown
Contributor

Thanks for this work! It was valuable research and after a lot of internal discussion we've agreed that while these subcommand docs (and our first use experience in general) could use improvement, this isn't the direction we'd like to go in. It's overly mechanical and, when we start looking into internationalization, would be difficult to translate.

@vilmibm vilmibm closed this Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants