Added Support preview Team Sync API for GitHub Enterprise Cloud users #1212
Conversation
googlebot
added
the
cla: yes
There was a problem hiding this comment.
It's looking good, @vaibhavsingh97! Thank you.
Just a few changes, please, and then I think we will be ready for a first and second LGTM.
github/github.go
Outdated
| @@ -147,6 +147,9 @@ const ( | |||
|
|
|||
| // https://developer.github.com/changes/2019-04-11-pulls-branches-for-commit/ | |||
| mediaTypeListPullsOrBranchesForCommitPreview = "application/vnd.github.groot-preview+json" | |||
|
|
|||
| // https://developer.github.com/changes/2019-06-12-team-sync/ | |||
| mediaTypeTeamSync = "application/vnd.github.team-sync-preview+json" | |||
There was a problem hiding this comment.
Can you please add Preview to the name to make it mediaTypeTeamSyncPreview ?
github/teams.go
Outdated
| @@ -476,3 +476,88 @@ func (s *TeamsService) RemoveTeamProject(ctx context.Context, teamID int64, proj | |||
|
|
|||
| return s.client.Do(ctx, req, nil) | |||
| } | |||
|
|
|||
| // GroupObject reprents list of Idp Group. | |||
There was a problem hiding this comment.
Please change this to: // IDPGroupList represents a list of external identity provider (IDP) groups.
I know GitHub spells it IdP, but Go always uses ID for "identification", so let's stick to the Go standard here too.
😄
There was a problem hiding this comment.
should I also change the name of struct to IDPGroupList ?
github/teams.go
Outdated
| } | ||
|
|
||
| // Group represents an Idp group status. | ||
| type Group struct { |
There was a problem hiding this comment.
Please change this to: // IDPGroup represents an external identity provider (IDP) group.
github/teams.go
Outdated
| GroupDescription *string `json:"group_description,omitempty"` | ||
| } | ||
|
|
||
| // ListIDPGroupsInOrganization list IdP groups available in an organization. |
There was a problem hiding this comment.
Please change list IdP to lists IDP.
github/teams.go
Outdated
|
|
||
| // ListIDPGroupsInOrganization list IdP groups available in an organization. | ||
| // | ||
| // Github API docs: https://developer.github.com/v3/teams/team_sync/ |
There was a problem hiding this comment.
Please use the official capitalization for GitHub.
URL should be: https://developer.github.com/v3/teams/team_sync/#list-idp-groups-in-an-organization
github/teams.go
Outdated
|
|
||
| groups := new(GroupObject) | ||
| resp, err := s.client.Do(ctx, req, groups) | ||
|
|
There was a problem hiding this comment.
Please delete this blank line between the err and its test.
github/teams.go
Outdated
|
|
||
| // ListIDPGroupsForTeam list IdP groups connected to a team on GitHub. | ||
| // | ||
| // Github API docs: https://developer.github.com/v3/teams/team_sync/ |
There was a problem hiding this comment.
s/Github/GitHub/
URL should be: https://developer.github.com/v3/teams/team_sync/#list-idp-groups-for-a-team
github/teams.go
Outdated
| // CreateOrUpdateIDPGroupConnections creates, updates, or removes a connection between a team | ||
| // and an IdP group. | ||
| // | ||
| // Github API docs: https://developer.github.com/v3/teams/team_sync/ |
There was a problem hiding this comment.
s/Github/GitHub/
URL should be: https://developer.github.com/v3/teams/team_sync/#create-or-update-idp-group-connections
github/teams.go
Outdated
| } | ||
|
|
||
| // CreateOrUpdateIDPGroupConnections creates, updates, or removes a connection between a team | ||
| // and an IdP group. |
Codecov Report
@@ Coverage Diff @@
## master #1212 +/- ##
==========================================
- Coverage 73.39% 73.31% -0.08%
==========================================
Files 84 84
Lines 5949 5985 +36
==========================================
+ Hits 4366 4388 +22
- Misses 825 832 +7
- Partials 758 765 +7
Continue to review full report at Codecov.
|
Signed-off-by: Vaibhav Singh <[email protected]>
|
@gmlewis Done with changes, please review 😄 |
There was a problem hiding this comment.
Thank you, @vaibhavsingh97!
LGTM.
Awaiting second LGTM before merging.
|
Thank you, @gauntface! |
Fixes #1200