Skip to content

refactor!: Pass RepositoryMergeRequest and RepoMergeUpstreamRequest by value#4372

Open
JamBalaya56562 wants to merge 2 commits into
google:masterfrom
JamBalaya56562:refactor/3644-repository-merge-value-params
Open

refactor!: Pass RepositoryMergeRequest and RepoMergeUpstreamRequest by value#4372
JamBalaya56562 wants to merge 2 commits into
google:masterfrom
JamBalaya56562:refactor/3644-repository-merge-value-params

Conversation

@JamBalaya56562

@JamBalaya56562 JamBalaya56562 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

This continues the #3644 value-parameter refactor for the two request bodies in github/repos_merging.go.

Repositories.Merge (POST /repos/{owner}/{repo}/merges)

  • Merge now takes RepositoryMergeRequest by value.
  • The schema-required Base and Head become non-pointer string (dropping omitempty); the optional CommitMessage stays *string with omitempty. Matches the OpenAPI schema 1:1 (required: [base, head]).

Repositories.MergeUpstream (POST /repos/{owner}/{repo}/merge-upstream)

  • MergeUpstream now takes RepoMergeUpstreamRequest by value.
  • The schema-required Branch becomes a non-pointer string (required: [branch]).

Both types were removed from the paramcheck body-allowed-pointer-types allowlist in .golangci.yml, and accessors were regenerated.

Towards #3644

…rge`

BREAKING CHANGE: `Repositories.Merge` now takes `RepositoryMergeRequest` by value instead of by pointer, and its required `Base` and `Head` fields are now non-pointer `string` values.
@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 Jul 8, 2026
@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.51%. Comparing base (90a1849) to head (eeb1a2f).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4372   +/-   ##
=======================================
  Coverage   97.51%   97.51%           
=======================================
  Files         193      193           
  Lines       19526    19526           
=======================================
  Hits        19040    19040           
  Misses        269      269           
  Partials      217      217           

☔ 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.
If you could please add the succint "BREAKING CHANGE:" line to the description, that would be appreciated.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.

cc: @stevehipwell - @alexandear - @Not-Dhananjay-Mishra

Comment thread github/repos_merging.go Outdated
…MergeUpstream`

BREAKING CHANGE: `Repositories.MergeUpstream` now takes `RepoMergeUpstreamRequest` by value instead of by pointer, and its required `Branch` field is now a non-pointer `string`.
@JamBalaya56562 JamBalaya56562 changed the title refactor!: Pass RepositoryMergeRequest by value in Repositories.Merge refactor!: Pass RepositoryMergeRequest and RepoMergeUpstreamRequest by value Jul 9, 2026
@JamBalaya56562 JamBalaya56562 requested a review from alexandear July 9, 2026 10:07
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.

3 participants