Skip to content

Add gh issue list and gh issue view#43

Merged
probablycorey merged 6 commits intomasterfrom
gh-issue
Nov 6, 2019
Merged

Add gh issue list and gh issue view#43
probablycorey merged 6 commits intomasterfrom
gh-issue

Conversation

@probablycorey
Copy link
Copy Markdown
Contributor

@probablycorey probablycorey commented Oct 31, 2019

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 add cmd.SilenceUsage = true to the top of each method run. This works, but it is kind of annoying that we need to remember to add it. Ignore all this, mislav figured it out in https://github.com/github/gh-cli/pull/41/files

@probablycorey probablycorey requested a review from a team October 31, 2019 18:13
@probablycorey probablycorey self-assigned this Oct 31, 2019
Copy link
Copy Markdown
Contributor

@mislav mislav left a comment

Choose a reason for hiding this comment

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

Looks great. 🌮

Short: "List issues",
RunE: issueList,
},
&cobra.Command{
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.

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",
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.

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?

Suggested change
Use: "view issue-number",
Use: "view <issue-number>",

@@ -0,0 +1,39 @@
package command
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.

Nice extraction into a new file! 👍

command/issue.go Outdated
}

func issueList(cmd *cobra.Command, args []string) error {
cmd.SilenceUsage = true
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.

You can take these out now if you merge master 🔥

Title string
}

func Issues(client *Client, ghRepo Repo, currentUsername string) (*IssuesPayload, error) {
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.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think that makes total sense for the next PR that touches queries.

@probablycorey probablycorey merged commit f3011fb into master Nov 6, 2019
@probablycorey probablycorey deleted the gh-issue branch November 6, 2019 18:13
vilmibm pushed a commit that referenced this pull request Jun 29, 2021
Fixing issues in the pr
mislav pushed a commit that referenced this pull request Sep 28, 2021
Add note about gh extension
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