Skip to content
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

Support preview GraphQL API v4 Node IDs #1149

Merged
merged 1 commit into from Apr 9, 2019

Conversation

Projects
None yet
5 participants
@vaibhavsingh97
Copy link
Contributor

vaibhavsingh97 commented Apr 9, 2019

Fixes #814

This pull request includes the GraphQL node ID in the response for the following REST API v3 resources:

  • Pull Requests
  • Repositories
  • Teams

@googlebot googlebot added the cla: yes label Apr 9, 2019

@codecov

This comment has been minimized.

Copy link

codecov bot commented Apr 9, 2019

Codecov Report

Merging #1149 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1149   +/-   ##
=======================================
  Coverage   70.18%   70.18%           
=======================================
  Files          84       84           
  Lines        5805     5805           
=======================================
  Hits         4074     4074           
  Misses        949      949           
  Partials      782      782
Impacted Files Coverage Δ
github/pulls_reviewers.go 58.62% <ø> (ø) ⬆️
github/teams.go 70.05% <ø> (ø) ⬆️
github/pulls_comments.go 64.38% <ø> (ø) ⬆️
github/pulls_reviews.go 78.57% <ø> (ø) ⬆️
github/repos_statuses.go 65% <ø> (ø) ⬆️
github/repos_commits.go 71.83% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c22c8e3...6c01ba5. Read the comment docs.

@gmlewis

gmlewis approved these changes Apr 9, 2019

Copy link
Collaborator

gmlewis left a comment

Thank you, @vaibhavsingh97!
LGTM.

Awaiting second LGTM before merging.

@gmlewis gmlewis requested a review from gauntface Apr 9, 2019

@gmlewis

This comment has been minimized.

Copy link
Collaborator

gmlewis commented Apr 9, 2019

Thank you, @gauntface!

Merging.

@gmlewis gmlewis merged commit d5a4c11 into google:master Apr 9, 2019

4 checks passed

cla/google All necessary CLAs are signed
codecov/patch Coverage not affected when comparing c22c8e3...6c01ba5
Details
codecov/project 70.18% remains the same compared to c22c8e3
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@vaibhavsingh97 vaibhavsingh97 deleted the vaibhavsingh97:patch/graphqlv4 branch Apr 10, 2019

@@ -121,6 +123,7 @@ func (s *TeamsService) GetTeam(ctx context.Context, team int64) (*Team, *Respons

// NewTeam represents a team to be created or modified.
type NewTeam struct {
NodeID *string `json:"node_id,omitempty"`

This comment has been minimized.

Copy link
@dmitshur

dmitshur Apr 14, 2019

Member

As far as I can tell, this field doesn't need to be added to NewTeam. NewTeam is used when creating or editing a team, but neither of those two endpoints accept a node_id parameter:

Only the response does, but struct Team corresponds to that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.