Skip to content

Conversation

areed
Copy link
Contributor

@areed areed commented Jul 6, 2017

Subcommands are implemented as methods on runners type, which holds
dependencies.
Integration tests with ginkgo run against an API.

Subcommands are implemented as methods on runners type, which holds
dependencies.
Integration tests with ginkgo run against an API.
@areed areed requested a review from emosbaugh July 6, 2017 02:25
Copy link
Member

@emosbaugh emosbaugh left a comment

Choose a reason for hiding this comment

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

This is awesome! I see no reason not to merge and continue to iterate on this.

One thing I'm concerned about is consistency in usage (use, short, and long).

Another thing I'd love to see is better godoc around the client package so its more useful for other devs. I'm not sure if the fact that its generated will make it useless. May be a reason to commit generated code to the repo? A link to the godoc in readme would be good too.

A cool thing we did with replicated on-prem cli is cobra allows docs generation from usage. See https://github.com/replicatedcom/replicated/blob/ed696f2af241a28619da514462739b94fe5ed897/Makefile#L79-L81. Possibly this and client godoc would be good additions to reference section of docs. https://www.replicated.com/docs/reference/

runCmds.api = api

if appSlug == "" {
appSlug = os.Getenv("REPLICATED_APP_SLUG")
Copy link
Member

Choose a reason for hiding this comment

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

it would be nice to be able to set id or slug

// releaseCmd represents the release command
var releaseCmd = &cobra.Command{
Use: "release",
Short: "manage app releases",
Copy link
Member

Choose a reason for hiding this comment

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

caps


var releaseCreateCmd = &cobra.Command{
Use: "create",
Short: "create a new release",
Copy link
Member

Choose a reason for hiding this comment

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

caps

// releaseInspectCmd represents the inspect command
var releaseInspectCmd = &cobra.Command{
Use: "inspect",
Short: "replicated release inspect <sequence>",
Copy link
Member

Choose a reason for hiding this comment

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

Print the YAML config for a release

var updateReleaseYaml string

var releaseUpdateCmd = &cobra.Command{
Use: "update SEQUENCE",
Copy link
Member

Choose a reason for hiding this comment

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

make use consistent
see promote <sequence> <channelID>

var channelRmCmd = &cobra.Command{
Use: "rm <id>",
Short: "Remove (archive) a channel",
Long: `replicated channel rm 4d3d240ea1ec4dab0be3b2105ff4b4ed`,
Copy link
Member

Choose a reason for hiding this comment

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

lets remove long descriptions that echo command

channelCmd.AddCommand(channelInspectCmd)
}

func (r *runners) channelInspect(cmd *cobra.Command, args []string) error {
Copy link
Member

Choose a reason for hiding this comment

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

im concerned about how parseable this output is. a lot of ppl will use cli for automation

return err
}

return print.Channels(r.w, allChannels)
Copy link
Member

Choose a reason for hiding this comment

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

would be awesome if printers supported formats eg tabs, json, yaml

var channelInspectCmd = &cobra.Command{
Use: "inspect",
Short: "Show full details for a channel",
Long: `replicated channel inspect be52315888f23408e2e4dc9242d4cc2c`,
Copy link
Member

Choose a reason for hiding this comment

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

lets remove long descriptions that echo command

@areed areed merged commit 3597440 into replicatedhq:master Jul 6, 2017
@areed
Copy link
Contributor Author

areed commented Jul 6, 2017

I fixed the short/long/use help text. Still todo:

  • docs from godoc and generated like on-prem cli
  • print yaml or json
  • accept appID in addition to slug
  • split channel inspect into multiple commands with more parseable output

@emosbaugh
Copy link
Member

@areed wanna create issues in github?

@areed
Copy link
Contributor Author

areed commented Jul 6, 2017

added

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.

2 participants