-
-
Notifications
You must be signed in to change notification settings - Fork 103
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
cmd: Add Issue show command #66
Conversation
Codecov Report
@@ Coverage Diff @@
## master #66 +/- ##
==========================================
+ Coverage 55.83% 56.64% +0.81%
==========================================
Files 21 22 +1
Lines 969 1038 +69
==========================================
+ Hits 541 588 +47
- Misses 287 299 +12
- Partials 141 151 +10
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall, few small comments
cmd/issueShow.go
Outdated
|
||
var issueShowCmd = &cobra.Command{ | ||
Use: "show [remote]", | ||
ArgAliases: []string{"s", "d"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a regular alias for get
Aliases: []string{"get"}
Also, consider dropping "d"
as a short alias "d"
makes me think "delete" over "describe".
assignee = issue.Assignee.Username | ||
} | ||
|
||
fmt.Printf(` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a template, but I'm tempted to say we can follow up on it.
Sample: https://github.com/zaquestion/lab/blob/master/cmd/issueCreate.go#L71
cmd/issueShow.go
Outdated
if err != nil { | ||
log.Fatal(err) | ||
} | ||
remote, _, err := parseArgsRemote(args) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The beginning of this function can just be
remote, issueNumber, err := parseArgsRemote(args)
cmd/issue.go
Outdated
}, | ||
} | ||
|
||
func init() { | ||
issueCmd.Flags().BoolP("list", "l", false, "list issues") | ||
issueCmd.Flags().BoolP("create", "c", false, "create new issue") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we drop the create flag out of this pr? I wanna think about it more.
7042854
to
b1bd06d
Compare
b1bd06d
to
a057d77
Compare
No description provided.