Add gh issue list and gh issue view#43
Conversation
| Short: "List issues", | ||
| RunE: issueList, | ||
| }, | ||
| &cobra.Command{ |
There was a problem hiding this comment.
Could we save these commands in named variables in-line with issueCmd, e.g. issueListCmd and issueViewCmd? This makes it easier to reference them later when we need to define flags on them
command/issue.go
Outdated
| RunE: issueList, | ||
| }, | ||
| &cobra.Command{ | ||
| Use: "view issue-number", |
There was a problem hiding this comment.
Could issue-number be in angle brackets so that it's clear that it's a variable and not a literal value to be passed?
| Use: "view issue-number", | |
| Use: "view <issue-number>", |
| @@ -0,0 +1,39 @@ | |||
| package command | |||
There was a problem hiding this comment.
Nice extraction into a new file! 👍
command/issue.go
Outdated
| } | ||
|
|
||
| func issueList(cmd *cobra.Command, args []string) error { | ||
| cmd.SilenceUsage = true |
There was a problem hiding this comment.
You can take these out now if you merge master 🔥
| Title string | ||
| } | ||
|
|
||
| func Issues(client *Client, ghRepo Repo, currentUsername string) (*IssuesPayload, error) { |
There was a problem hiding this comment.
Since queries.go will get huge really fast like this, maybe it's time to consider splitting it into multiple files per topic? E.g. queries_issue.go, queries_checks.go, etc.
There was a problem hiding this comment.
I think that makes total sense for the next PR that touches queries.
This takes the prototype code for issues and applies the changes @mislav recommended in https://github.com/github/gh-cli/pull/33/files. It also uses the new test pattern and context patterns which worked out VERY WELL.
One thing to note. I looked into the annoying problem of the usage being output whenever a command has an error. Turns out other people agree with us spf13/cobra#340! In order to still output the usage when the command formatting is wrong I had to addIgnore all this, mislav figured it out in https://github.com/github/gh-cli/pull/41/filescmd.SilenceUsage = trueto the top of each method run. This works, but it is kind of annoying that we need to remember to add it.