Skip to content

refactor!: Pass the Teams body types by value and rename them to ...Request - #4606

Open
JamBalaya56562 wants to merge 2 commits into
google:masterfrom
JamBalaya56562:refactor/3644-teams-request-types
Open

JamBalaya56562 wants to merge 2 commits into
google:masterfrom
JamBalaya56562:refactor/3644-teams-request-types

Conversation

@JamBalaya56562

Copy link
Copy Markdown
Contributor

Updates #3644

This converts the three TeamsService body types, removing 6 entries from the paramcheck exception lists in .golangci.yml (3 from body-allowed-pointer-types and 3 from body-allowed-wrong-names), plus the 2 structfield allowed-tag-types entries marked # TODO: Teams.

Old New Methods
TeamAddTeamMembershipOptions AddTeamMembershipRequest AddTeamMembershipByID, AddTeamMembershipBySlug
TeamAddTeamRepoOptions AddTeamRepoRequest AddTeamRepoByID, AddTeamRepoBySlug
TeamProjectOptions AddTeamProjectRequest AddTeamProjectByID, AddTeamProjectBySlug

The first commit makes TeamAddTeamMembershipOptions.Role and TeamAddTeamRepoOptions.Permission *string, which clears the two # TODO: Teams entries in structfield's allowed-tag-types. Both properties are optional in the OpenAPI request schemas (neither schema has a required list), and TeamProjectOptions.Permission was already *string.

The method names are kept as Add...: the docs titles are "Add or update ...", and Add already matches the verb these methods have always used.

BREAKING CHANGE: TeamAddTeamMembershipOptions, TeamAddTeamRepoOptions and TeamProjectOptions are renamed to AddTeamMembershipRequest, AddTeamRepoRequest and AddTeamProjectRequest; TeamsService.AddTeamMembershipByID, AddTeamMembershipBySlug, AddTeamRepoByID, AddTeamRepoBySlug, AddTeamProjectByID and AddTeamProjectBySlug now take them by value instead of by pointer; AddTeamMembershipRequest.Role and AddTeamRepoRequest.Permission are now *string.

@gmlewis gmlewis added NeedsReview PR is awaiting a review before merging. Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s). labels Oct 4, 2026
@codecov

codecov Bot commented Oct 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.60%. Comparing base (52908e2) to head (c3e5381).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4606   +/-   ##
=======================================
  Coverage   98.60%   98.60%           
=======================================
  Files         198      198           
  Lines       18552    18552           
=======================================
  Hits        18294    18294           
  Misses        258      258           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gmlewis gmlewis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @JamBalaya56562!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.

cc: @stevehipwell - @Not-Dhananjay-Mishra

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s). NeedsReview PR is awaiting a review before merging.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants