Conversation
6e77988 to
bced6e7
Compare
api/queries_issue.go
Outdated
| } | ||
|
|
||
| func IssueList(client *Client, repo ghrepo.Interface, state string, labels []string, assigneeString string, limit int, authorString string) (*IssuesAndTotalCount, error) { | ||
| func IssueList(client *Client, repo ghrepo.Interface, state string, labels []string, assigneeString string, limit int, authorString string, mentionedString string, milestoneString string) (*IssuesAndTotalCount, error) { |
There was a problem hiding this comment.
Don't want to sound picky but don't you guys think it's a little redundant to have the String suffix in the arg name?
Like in mentionedString, assigneeString
There was a problem hiding this comment.
Agreed that it's starting to look weird, but I don't think this is something that we need to address in this PR. As a follow-up, we will look into having IssueList accept a struct of filtering options or something similar, rather than a big argument list that starting to be hard to maintain.
@eddumelendez Thank you for this PR! The code looks good; we're just holding off merging features in general until we figure out things like exact flag naming and how we can fit these features in our release schedule ✨
|
Just a question, but how would one retrieve the milestone number to use in this command? I was hoping to pass the milestone title but that doesn't appear to work based on the docs. The milestone number also doesn't work. It looks like one would need to decode the ID and use that number. Please tell me I'm looking at this wrong. |
probablycorey
left a comment
There was a problem hiding this comment.
@eddumelendez I tested this out locally but got this error:
graphql error: 'Variable $mentioned is used by but not declared, Variable $milestone is used by but not declared'
I think it is because the milestone and mentioned vars need to be defined in the query here https://github.com/cli/cli/pull/644/files#diff-f9ae6189655de6363f9ac71892daec02R215.
@mislav once this works do you think we can move forward with it, or do you still want to figure out the flag naming scheme you mentioned in https://github.com/cli/cli/pull/644/files#r395592397
48b599f to
a164056
Compare
|
thank you so much @probablycorey good catch! I have rebased the branch and add the fix |
mislav
left a comment
There was a problem hiding this comment.
@probablycorey This looks good to me! One tiny nit about the flag name though
command/issue.go
Outdated
| issueListCmd.Flags().StringP("state", "s", "open", "Filter by state: {open|closed|all}") | ||
| issueListCmd.Flags().IntP("limit", "L", 30, "Maximum number of issues to fetch") | ||
| issueListCmd.Flags().StringP("author", "A", "", "Filter by author") | ||
| issueListCmd.Flags().StringP("mentioned", "", "", "Filter by mention") |
There was a problem hiding this comment.
I'm not 100% sold on "mentioned" since it's past tense. How about present tense, e.g. --mentions ampinsk? @ampinsk: would love to know your thougths.
There was a problem hiding this comment.
I have updated the flag name
7c2f3c5 to
bb29099
Compare
bb29099 to
cffd56f
Compare
addressed GraphQL error, flag naming
| query := fragments + ` | ||
| query($owner: String!, $repo: String!, $limit: Int, $endCursor: String, $states: [IssueState!] = OPEN, $labels: [String!], $assignee: String, $author: String) { | ||
| query($owner: String!, $repo: String!, $limit: Int, $endCursor: String, $states: [IssueState!] = OPEN, $labels: [String!], $assignee: String, $author: String, $mention: String, $milestone: String) { | ||
| repository(owner: $owner, name: $repo) { |
This commit introduces two new flags in order to filter issues per
mentionedandmilestone.See #641