refactor!: Pass issue request by value and split `CreateIssueRequest` from `IssueRequest` by jvm986 · Pull Request #4396 · google/go-github

jvm986 · 2026-07-18T12:11:08Z

BREAKING CHANGE: IssueService.Edit is renamed to IssueService.Update to match the underlying "update an issue" endpoint.

IssuesService.Create now takes a dedicated CreateIssueRequest by value (with a required non-pointer Title), and IssuesService.Updated takes IssueRequest by value instead of by pointer.

Towards #3644.

…dit` Towards google#3644. BREAKING CHANGE: `IssuesService.Create` and `IssuesService.Edit` now take `IssueRequest` by value instead of by pointer.

codecov · 2026-07-18T12:57:29Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.52%. Comparing base (f15206a) to head (43587ee).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4396   +/-   ##
=======================================
  Coverage   97.52%   97.52%           
=======================================
  Files         193      193           
  Lines       19668    19668           
=======================================
  Hits        19182    19182           
  Misses        268      268           
  Partials      218      218

☔ 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

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

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

Oh, and @jvm986 and @JamBalaya56562 - feel free to "cc:" each other on each PR to be extra careful that work is not duplicated.

jvm986 · 2026-07-18T13:09:55Z

@gmlewis, @JamBalaya56562

What are your thoughts on the CreateIssueRequest question?

IssueRequest is shared by Create (API requires title) and Edit (all optional), so I only changed the param to a value — promoting Title to a required string would break Edit. Should we split into a dedicated CreateIssueRequest (like #4382 did for deployment branch policies), or is the shared type fine here?

gmlewis · 2026-07-18T13:13:31Z

@gmlewis, @JamBalaya56562

What are your thoughts on the CreateIssueRequest question?

IssueRequest is shared by Create (API requires title) and Edit (all optional), so I only changed the param to a value — promoting Title to a required string would break Edit. Should we split into a dedicated CreateIssueRequest (like #4382 did for deployment branch policies), or is the shared type fine here?

I think there was a discussion for #4382 where the change of name was deemed valuable, so I think we should also address that here in the breaking change PR to remain consistent.

Thank you for bringing this up! I appreciate it.

Towards google#3644. BREAKING CHANGE: `IssuesService.Create` now takes a dedicated `CreateIssueRequest` with a required non-pointer `Title`, instead of the shared `IssueRequest`. The create and edit operations have different request contracts: the create API requires `title` and does not accept `state`/`state_reason`, whereas edit treats all fields as optional. The new `CreateIssueRequest` enforces the required `Title` at compile time and drops the edit-only fields. `CreateIssueRequest.Labels` and `.Assignees` use `[]string` (not `*[]string`), since an explicit empty array is only meaningful for edit.

Not-Dhananjay-Mishra · 2026-07-18T13:59:33Z

+// IssueRequest represents a request to edit an issue.
 // It is separate from Issue above because otherwise Labels
 // and Assignee fail to serialize to the correct JSON.
 type IssueRequest struct {


Can we update IssueRequest struct too.
It has one missing field duplicate_issue_id and we should use []string instead of *[]string for Labels and Assignees

Nice find on duplicate_issue_id, will add it.

Regarding Labels and Assignees the logic here is that the update endpoint treats "labels": [] as "clear all labels" (distinct from omitting the field which leaves them unchanged). []string isn't able to distinguish between omission and an explicit empty array.

@gmlewis could you help me with this? I assumed that we don't use *[]buildin in this repo.

Regarding Labels and Assignees the logic here is that the update endpoint treats "labels": [] as "clear all labels" (distinct from omitting the field which leaves them unchanged). []string isn't able to distinguish between and an explicit empty array.

omitzero seems like a good fit here instead of omitempty, since it omits only nil slices while preserving an explicit empty slice.

omitzero seems like a good fit here instead of omitempty, since it omits only nil slices while preserving an explicit empty slice.

Yes, exactly right, @Not-Dhananjay-Mishra - this is what omitzero is for. So we can finally get rid of the *[]string, change it to []string and then use omitzero on it so that nil is different from []string{}.

Thanks @gmlewis.
@jvm986, you can change *[]string to []string for Labels and Assignees, and use omitzero instead of omitempty.

Thanks @gmlewis. @jvm986, you can change *[]string to []string for Labels and Assignees, and use omitzero instead of omitempty.

Yes, please, and just be absolutely sure to include a table-driven unit test where BOTH nil and []string{} are passed to ensure that the generated JSON matches our expections, and preferrably add a comment that explains when to use one versus the other. Thank you, @jvm986!

Not-Dhananjay-Mishra · 2026-07-18T14:03:12Z

 //
 //meta:operation PATCH /repos/{owner}/{repo}/issues/{issue_number}
-func (s *IssuesService) Edit(ctx context.Context, owner, repo string, number int, body *IssueRequest) (*Issue, *Response, error) {
+func (s *IssuesService) Edit(ctx context.Context, owner, repo string, number int, body IssueRequest) (*Issue, *Response, error) {


Can we rename this to Update? as API docs mentioned it as "Update an issue"

Makes sense to me, will update after feedback from others.

Makes sense to me, will update after feedback from others.

SGTM. Thank you, @Not-Dhananjay-Mishra and @jvm986!

I figure that while we are actually breaking the API, we should always attempt to make it as easy-to-understand and easy-to-use as possible, and this seems to fit into that category. 😄

Towards google#3644. The update-an-issue endpoint accepts `duplicate_issue_id`, which is required when `state_reason` is `duplicate`. Add the field to `IssueRequest` and update the `StateReason` comment to list the `duplicate` and `reopened` values.

Towards google#3644. BREAKING CHANGE: `IssuesService.Edit` is renamed to `IssuesService.Update`. The method name now matches the underlying "update an issue" endpoint and GitHub's own naming, aligning it with the rest of the service

Towards google#3644. BREAKING CHANGE: `IssueRequest.Labels` and `IssueRequest.Assignees` change from `*[]string` to `[]string`. The update endpoint distinguishes omitting a field (leave unchanged) from sending an empty array (clear all). `[]string` with `omitzero` captures.

alexandear · 2026-07-20T12:46:31Z

-	// StateReason can be 'completed' or 'not_planned'.
-	StateReason      *string                   `json:"state_reason,omitempty"`
+type CreateIssueRequest struct {
+	// Title is required when creating an issue.


This comment can be removed.

alexandear · 2026-07-20T12:47:20Z

 //
 //meta:operation PATCH /repos/{owner}/{repo}/issues/{issue_number}
-func (s *IssuesService) Edit(ctx context.Context, owner, repo string, number int, body *IssueRequest) (*Issue, *Response, error) {
+func (s *IssuesService) Update(ctx context.Context, owner, repo string, number int, body IssueRequest) (*Issue, *Response, error) {


Let's rename this as well:

Suggested change

func (s *IssuesService) Update(ctx context.Context, owner, repo string, number int, body IssueRequest) (*Issue, *Response, error) {

func (s *IssuesService) Update(ctx context.Context, owner, repo string, number int, body UpdateIssueRequest) (*Issue, *Response, error) {

Good call, as it's a breaking change anyway.

Towards: google#3644 Remove redundant comment.

refactorPass IssueRequest by value in Issues.Create and `Issues.E…

7b1f980

…dit` Towards google#3644. BREAKING CHANGE: `IssuesService.Create` and `IssuesService.Edit` now take `IssueRequest` by value instead of by pointer.

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 18, 2026

gmlewis approved these changes Jul 18, 2026

View reviewed changes

jvm986 added 2 commits July 18, 2026 15:41

Merge branch 'master' into issues-create-edit-by-value

17501b9

Not-Dhananjay-Mishra reviewed Jul 18, 2026

View reviewed changes

jvm986 changed the title ~~refactor!: Pass IssueRequest by value in Issues.Create and Issues.Edit~~ refactor!: Pass issue request by value and split CreateIssueRequest from IssueRequest Jul 18, 2026

jvm986 added 3 commits July 19, 2026 11:58

refactor!: Rename Issues.Edit to Issues.Update

405357e

Towards google#3644. BREAKING CHANGE: `IssuesService.Edit` is renamed to `IssuesService.Update`. The method name now matches the underlying "update an issue" endpoint and GitHub's own naming, aligning it with the rest of the service

Merge branch 'master' into issues-create-edit-by-value

420dee0

alexandear reviewed Jul 20, 2026

View reviewed changes

jvm986 added 2 commits July 20, 2026 15:55

Merge branch 'master' into issues-create-edit-by-value

64c94f7

refactor!: Rename IssueRequest to UpdateIssueRequest

43587ee

Towards: google#3644 Remove redundant comment.

	func (s IssuesService) Update(ctx context.Context, owner, repo string, number int, body IssueRequest) (Issue, *Response, error) {
	func (s IssuesService) Update(ctx context.Context, owner, repo string, number int, body UpdateIssueRequest) (Issue, *Response, error) {

Uh oh!

Conversation

jvm986 commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gmlewis left a comment

Choose a reason for hiding this comment

Uh oh!

jvm986 commented Jul 18, 2026

Uh oh!

gmlewis commented Jul 18, 2026

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!

jvm986 Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jvm986 commented Jul 18, 2026 •

edited

Loading

codecov Bot commented Jul 18, 2026 •

edited

Loading

jvm986 Jul 18, 2026 •

edited

Loading