-
Notifications
You must be signed in to change notification settings - Fork 48
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
ExampleGQLClient_Mutate_simple #99
Conversation
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 great; thank you! Few nitpicks
example_gh_test.go
Outdated
} | ||
variables := map[string]interface{}{ | ||
"input": githubv4.AddStarInput{ | ||
ClientMutationID: githubv4.NewString("addStar"), |
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.
Let's remove ClientMutationID
since it's not necessary for the request
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.
my intent with ClientMutationID was to show how to convert inputs, but it looks like that works for StarrableID as well 😄
example_gh_test.go
Outdated
Stargazers struct { | ||
Nodes []struct { | ||
Login string |
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.
Why query the information about login names for the latest 10 stargazers? I don't feel that helps the example, and instead just adds noise. Maybe we could just fetch StargazerCount like for gists and display that as the result of the mutation
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.
I have come across this somewhat regularly when using the apis, for example get the issues in a ProjectsV2 or the comments in a PullRequest, and wanted to showcase how that is used in a mutation. I do see that there are existing examples of it in ExampleGQLClient_simple
and ExampleGQLClient_advanced
, I'll remove it here
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.
oh - I also wanted to show the use of first
, separate from input
, which I don't see in other examples and is often required by the api. I removed it in a77f72b if you prefer without it, or we can revert it if you think it adds something
example_gh_test.go
Outdated
} `graphql:"... on Repository"` | ||
Gist struct { | ||
StargazerCount int | ||
Url string |
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 Url
field should be called URL
according to Go field naming conventions, but I'd say lets omit it unless we are going to consume the value
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.
👍
Thank you! |
Fixes #98
Adds a mutation for https://docs.github.com/en/graphql/reference/mutations#addstar with input and first params, and inline fragments on the response to demonstrate how these are used.
I tested this in a local
func main()
to confirm the format is correct and the request can succeed (output isvilmibm
).The node that global_id
R_kgDOF_MgQQ
references is left as an exercise for the user 😄