Move the GitHub Teams API #1400

sprsld · 2020-01-28T04:27:38Z

Fixes: #1387.

… general teams

weeco · 2020-01-28T18:05:51Z

I think this PR is missing (some?) of the new Team API endpoints e. g. fetching a team by it's slug GET /orgs/:org/teams/:team_slug, see: https://developer.github.com/changes/2020-01-21-moving-the-team-api-endpoints/

gmlewis · 2020-01-29T01:55:54Z

Thank you, @weeco - I've made a list in #1387 so we can hopefully keep track of the changes needed, and the ones that are implemented, as we review the PR(s).

sprsld · 2020-01-29T02:22:06Z

@weeco - I will go ahead and take a look again. As for the particular endpoint you referenced the implementation for that should be in github/teams.go on line 119. Is that what you are referring to?

gmlewis

Thanks, @Atorr, but this PR is attempting to do way too much and can not be fully reviewed as written.
As described below, please address the comments reviewed so far, and revert all the unnecessary changes performed in reorganizing the file structure.

Also, please to not use "force push" for any future commits, as that will waste a great deal of time when reviewing. Thanks.

gmlewis · 2020-01-29T01:57:30Z

github/teams.go

-// GitHub API docs: https://developer.github.com/v3/teams/#get-team
-func (s *TeamsService) GetTeam(ctx context.Context, team int64) (*Team, *Response, error) {
-	u := fmt.Sprintf("teams/%v", team)
+// Github API docs: https://developer.github.com/v3/teams/#get-team-by-name


s/Github/GitHub/

gmlewis · 2020-01-29T02:02:49Z

github/teams.go

@@ -239,11 +239,38 @@ func (s *TeamsService) EditTeam(ctx context.Context, id int64, team NewTeam, rem
 	return t, resp, nil
 }

-// DeleteTeam deletes a team.
+// EditTeamByName edits a team, given an organiation name, by name.


For consistency, please rename this function to EditTeamBySlug and change comment to say "by slug."

gmlewis · 2020-01-29T02:03:43Z

github/teams.go

+// DeleteTeamByName deletes a team reference by Name.
+//
+// GitHub API docs: https://developer.github.com/v3/teams/#delete-team
+func (s *TeamsService) DeleteTeamByName(ctx context.Context, org, slug string) (*Response, error) {


For consistency, please rename this function to DeleteTeamBySlug and change comment to day "reference by slug."

gmlewis · 2020-01-29T02:06:01Z

github/teams.go

@@ -276,11 +316,63 @@ func (s *TeamsService) ListChildTeams(ctx context.Context, teamID int64, opt *Li
 	return teams, resp, nil
 }

-// ListTeamRepos lists the repositories that the specified team has access to.
+// ListChildTeamsByParentName lists child teams for a parent team given parent name.


For consistency, please rename this function to ListChildTeamsByParentSlug, and change the comment to say "given parent slug".

gmlewis · 2020-01-29T02:09:28Z

github/teams.go

+	return repos, resp, nil
+}
+
+// ListTeamReposByName lists the repositories given a team name that the specified team has access to.


ListTeamReposBySlug... given a team slug.

gmlewis · 2020-01-29T02:16:27Z

github/teams.go

+	return s.client.Do(ctx, req, nil)
+}
+
+// RemoveTeamRepoByName removes a repository from being managed by the specified


RemoveTeamRepoBySlug...given the team slug.

gmlewis · 2020-01-29T02:17:26Z

github/teams.go

+	return projects, resp, nil
+}
+
+// ListTeamProjectsByName lists the organization projects for a team given the team name.


ListTeamProjectsBySlug... given the team slug.

gmlewis · 2020-01-29T02:19:10Z

github/teams.go

-// as an organization member, the authenticated user must have "read" access to both the team
-// and project, or "admin" access to the team or project.
-// Note: This endpoint removes the project from the team, but does not delete it.
+// AddTeamProjectByName adds an organization project to a team given the team name.


AddTeamProjectBySlug...given the team slug.

gmlewis · 2020-01-29T02:20:53Z

github/teams.go

 }

-// CreateOrUpdateIDPGroupConnections creates, updates, or removes a connection between a team
-// and an IDP group.
+// RemoveTeamProjectByName removes an organization project from a team given team name.


RemoveTeamProjectBySlug...given team slug.

gmlewis · 2020-01-29T02:23:18Z

github/teams.go

@@ -494,77 +686,46 @@ func (s *TeamsService) RemoveTeamProject(ctx context.Context, teamID int64, proj
 	return s.client.Do(ctx, req, nil)
 }

-// IDPGroupList represents a list of external identity provider (IDP) groups.


Why are lines 497 to the end of the file being deleted in this PR?
Oh, it looks like you are also trying to reorganize the files in this PR as well?
Sorry, but please handle file reorganization in a separate PR.

Attempting to address the changes listed in the announcement: https://developer.github.com/changes/2020-01-21-moving-the-team-api-endpoints/
while at the same time reorganizing files and moving code around makes this PR much much larger than it needs to be and is not reviewable as such.

So please keep this PR focused just on the checklist I made in #1387 and revert the rest of the changes and make a separate PR for those. Thanks.

Sure thing. Must've missed the list. Working on reverting the changes now. Thanks!

…m members and team synchronization endpoints, and make fixes per PR review

sprsld · 2020-01-29T04:06:15Z

New changes in. Had missed the link to the blog post and went a bit overboard. Will work to pay more attention to what needs to be changed. Thank you all for your patience on this!

gmlewis

Hey @Atorr ... this looks fantastic now!

Thank you so much for all this work you've done! It is tremendously appreciated by myself and by the hundreds (or maybe even thousands) of happy, silent users of this client library! 😄

LGTM.

Awaiting second LGTM before merging.

weeco · 2020-02-03T19:53:18Z

Is there any chance to get that merged/released soonish?

gmlewis · 2020-02-04T23:58:16Z

Hi @weeco - this repo is maintained by volunteers and it is difficult to tell when people will have time to review PRs, so I don't have an ETA for you.

I suggest that if you need something right away, you fork the repo and pull in the PR(s) that you need and work off of that.

Thank you for your patience.

wesleimp

👌

gmlewis · 2020-02-10T13:19:47Z

Thank you, @wesleimp!
Merging.

coilysiren · 2020-02-10T19:16:15Z

👋 @gmlewis I have a request 🙏 can you post a comment in this PR when there's a release that includes this change? I'd appreciate it tons ✨

gmlewis · 2020-02-10T19:32:56Z

@lynncyrin - https://github.com/google/go-github/releases/tag/v29.0.3 incorporates this change.

Fixes: google#1387.

sprsld added 9 commits January 25, 2020 12:37

fix: update general teams endpoints

6d81562

fix: update team members endpoints

95f39e2

fix: add implementation for using team and org IDs

1809f4b

fix: teams members functions added to get by ID, fix ID endpoints for…

61f02c6

… general teams

fix: add functions for team discussion comments endpoints

eddabd2

fix: add functions for team discussion endpoints

5307f59

style: linting and formatting fixes

591d90d

update teams synchronization endpoints and house them in separate file

03ec694

fix: remove deprecated functions

60a41cf

googlebot added the cla: yes Indication that the PR author has signed a Google Contributor License Agreement. label Jan 28, 2020

sprsld requested a review from gmlewis January 28, 2020 04:32

gmlewis requested changes Jan 29, 2020

View reviewed changes

fix: revert changes to team discussion, team discussion comments, tea…

3cda28d

…m members and team synchronization endpoints, and make fixes per PR review

gmlewis approved these changes Jan 29, 2020

View reviewed changes

gmlewis requested a review from gauntface January 29, 2020 04:16

wesleimp approved these changes Feb 10, 2020

View reviewed changes

Merge branch 'master' into issue-1387

0fc4796

gmlewis merged commit 2e3e74f into google:master Feb 10, 2020

Hsn723 mentioned this pull request Feb 11, 2020

Bump github.com/google/go-github/v29 from 29.0.2 to 29.0.3 MatsuriJapon/git-matsuri#37

Merged

This was referenced Feb 12, 2020

Update teams_members.go #1422

Closed

Update teams_discussion_comments.go #1423

Closed

gmlewis mentioned this pull request Feb 12, 2020

Update teams_discussions.go #1424

Closed

benj-fletch mentioned this pull request Feb 28, 2020

Updated to go-github v29.0.3 integrations/terraform-provider-github#369

Merged

n1lesh pushed a commit to n1lesh/go-github that referenced this pull request Oct 2, 2020

Move the GitHub Teams API (google#1400)

a4527ce

Fixes: google#1387.

Move the GitHub Teams API #1400

Move the GitHub Teams API #1400

Uh oh!

Conversation

sprsld commented Jan 28, 2020 • edited by gmlewis Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

weeco commented Jan 28, 2020

Uh oh!

gmlewis commented Jan 29, 2020

Uh oh!

sprsld commented Jan 29, 2020

Uh oh!

gmlewis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sprsld commented Jan 29, 2020

Uh oh!

gmlewis left a comment

Choose a reason for hiding this comment

Uh oh!

weeco commented Feb 3, 2020

Uh oh!

gmlewis commented Feb 4, 2020

Uh oh!

wesleimp left a comment

Choose a reason for hiding this comment

Uh oh!

gmlewis commented Feb 10, 2020

Uh oh!

coilysiren commented Feb 10, 2020

Uh oh!

gmlewis commented Feb 10, 2020

Uh oh!

Uh oh!

sprsld commented Jan 28, 2020 •

edited by gmlewis

Loading