refactor!: Pass release params by value and rename EditRelease to UpdateRelease#4329
Conversation
EditRelease to UpdateRelease
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4329 +/- ##
==========================================
- Coverage 97.48% 97.48% -0.01%
==========================================
Files 193 193
Lines 19440 19417 -23
==========================================
- Hits 18952 18929 -23
Misses 270 270
Partials 218 218 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
gmlewis
left a comment
There was a problem hiding this comment.
Thank you, @JamBalaya56562!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
alexandear
left a comment
There was a problem hiding this comment.
As we are making breaking changes, I propose creating separate structs for creating and updating a repository release. These structs have slightly different fields.
| return nil, nil, errors.New("release must be provided") | ||
| } | ||
|
|
||
| func (s *RepositoriesService) CreateRelease(ctx context.Context, owner, repo string, release RepositoryRelease) (*RepositoryRelease, *Response, error) { |
There was a problem hiding this comment.
| func (s *RepositoriesService) CreateRelease(ctx context.Context, owner, repo string, release RepositoryRelease) (*RepositoryRelease, *Response, error) { | |
| func (s *RepositoriesService) CreateRelease(ctx context.Context, owner, repo string, body CreateReleaseRequest) (*RepositoryRelease, *Response, error) { |
There was a problem hiding this comment.
Done in 69f00c3 — added a dedicated CreateReleaseRequest and CreateRelease now takes it by value. Thanks!
| return nil, nil, errors.New("release must be provided") | ||
| } | ||
|
|
||
| func (s *RepositoriesService) UpdateRelease(ctx context.Context, owner, repo string, id int64, release RepositoryRelease) (*RepositoryRelease, *Response, error) { |
There was a problem hiding this comment.
| func (s *RepositoriesService) UpdateRelease(ctx context.Context, owner, repo string, id int64, release RepositoryRelease) (*RepositoryRelease, *Response, error) { | |
| func (s *RepositoriesService) UpdateRelease(ctx context.Context, owner, repo string, id int64, body UpdateReleaseRequest) (*RepositoryRelease, *Response, error) { |
There was a problem hiding this comment.
Done — UpdateRelease now takes UpdateReleaseRequest by value. It omits GenerateReleaseNotes, since the update endpoint does not accept it.
There was a problem hiding this comment.
We can remove this after introducing CreateReleaseRequest and UpdateReleaseRequest
There was a problem hiding this comment.
Removed — the internal repositoryReleaseRequest is gone now that CreateReleaseRequest/UpdateReleaseRequest carry the request fields directly and are serialized as-is.
69f00c3 to
419cb63
Compare
There was a problem hiding this comment.
| testJSONBody(t, r, input) |
There was a problem hiding this comment.
Done — dropped the duplicated want and pass input directly (the handler closure captures it).
There was a problem hiding this comment.
| testJSONBody(t, r, input) |
There was a problem hiding this comment.
Done — same here, using input directly.
| DiscussionCategoryName *string `json:"discussion_category_name,omitempty"` | ||
|
|
||
| // The following fields are not used in EditRelease: | ||
| // The following fields are not used in UpdateRelease: |
There was a problem hiding this comment.
This comment is no longer needed:
| // The following fields are not used in UpdateRelease: |
There was a problem hiding this comment.
Done — removed the obsolete comment.
| GenerateReleaseNotes *bool `json:"generate_release_notes,omitempty"` | ||
|
|
||
| // The following fields are not used in CreateRelease or EditRelease: | ||
| // The following fields are not used in CreateRelease or UpdateRelease: |
There was a problem hiding this comment.
This comment is no longer needed:
| // The following fields are not used in CreateRelease or UpdateRelease: |
There was a problem hiding this comment.
Done — removed.
| Prerelease *bool `json:"prerelease,omitempty"` | ||
| // CreateReleaseRequest represents a request to create a release in a repository. | ||
| type CreateReleaseRequest struct { | ||
| TagName *string `json:"tag_name,omitempty"` |
There was a problem hiding this comment.
Done in b2563e2 — TagName is now a required string (json:"tag_name", no omitempty), since the create endpoint requires it. UpdateReleaseRequest.TagName stays optional.
| client, mux, _ := setup(t) | ||
|
|
||
| input := &RepositoryRelease{ | ||
| input := CreateReleaseRequest{ |
There was a problem hiding this comment.
Please fill required TagName.
There was a problem hiding this comment.
Done — set TagName: "v1.0" in the test input now that it is required.
419cb63 to
b2563e2
Compare
| @@ -29,10 +29,8 @@ | |||
| MakeLatest *string `json:"make_latest,omitempty"` | |||
| DiscussionCategoryName *string `json:"discussion_category_name,omitempty"` | |||
|
|
|||
There was a problem hiding this comment.
Removed — this blank line is gone now that the generate_release_notes block was dropped from RepositoryRelease (it's input-only and now lives in CreateReleaseRequest).
|
|
||
| // The following fields are not used in EditRelease: | ||
| GenerateReleaseNotes *bool `json:"generate_release_notes,omitempty"` | ||
|
|
There was a problem hiding this comment.
Removed as well, as part of the same change — the GenerateReleaseNotes field and its surrounding blank lines are gone now that the struct matches the response schema.
| ) | ||
|
|
||
| // RepositoryRelease represents a GitHub release in a repository. | ||
| type RepositoryRelease struct { |
There was a problem hiding this comment.
The struct should match the schema:
{
"title": "Release",
"description": "A release.",
"type": "object",
"properties": {
"url": {
"type": "string",
"format": "uri"
},
"html_url": {
"type": "string",
"format": "uri"
},
"assets_url": {
"type": "string",
"format": "uri"
},
"upload_url": {
"type": "string"
},
"tarball_url": {
"type": [
"string",
"null"
],
"format": "uri"
},
"zipball_url": {
"type": [
"string",
"null"
],
"format": "uri"
},
"id": {
"type": "integer"
},
"node_id": {
"type": "string"
},
"tag_name": {
"description": "The name of the tag.",
"type": "string"
},
"target_commitish": {
"description": "Specifies the commitish value that determines where the Git tag is created from.",
"type": "string"
},
"name": {
"type": [
"string",
"null"
]
},
"body": {
"type": [
"string",
"null"
]
},
"draft": {
"description": "true to create a draft (unpublished) release, false to create a published one.",
"type": "boolean"
},
"prerelease": {
"description": "Whether to identify the release as a prerelease or a full release.",
"type": "boolean"
},
"immutable": {
"description": "Whether or not the release is immutable.",
"type": "boolean"
},
"created_at": {
"type": "string",
"format": "date-time"
},
"published_at": {
"type": [
"string",
"null"
],
"format": "date-time"
},
"updated_at": {
"type": [
"string",
"null"
],
"format": "date-time"
},
"author": {
"title": "Simple User",
"description": "A GitHub user.",
"type": "object",
"properties": {
"name": {
"type": [
"string",
"null"
]
},
"email": {
"type": [
"string",
"null"
]
},
"login": {
"type": "string"
},
"id": {
"type": "integer",
"format": "int64"
},
"node_id": {
"type": "string"
},
"avatar_url": {
"type": "string",
"format": "uri"
},
"gravatar_id": {
"type": [
"string",
"null"
]
},
"url": {
"type": "string",
"format": "uri"
},
"html_url": {
"type": "string",
"format": "uri"
},
"followers_url": {
"type": "string",
"format": "uri"
},
"following_url": {
"type": "string"
},
"gists_url": {
"type": "string"
},
"starred_url": {
"type": "string"
},
"subscriptions_url": {
"type": "string",
"format": "uri"
},
"organizations_url": {
"type": "string",
"format": "uri"
},
"repos_url": {
"type": "string",
"format": "uri"
},
"events_url": {
"type": "string"
},
"received_events_url": {
"type": "string",
"format": "uri"
},
"type": {
"type": "string"
},
"site_admin": {
"type": "boolean"
},
"starred_at": {
"type": "string"
},
"user_view_type": {
"type": "string"
}
},
"required": [
"avatar_url",
"events_url",
"followers_url",
"following_url",
"gists_url",
"gravatar_id",
"html_url",
"id",
"node_id",
"login",
"organizations_url",
"received_events_url",
"repos_url",
"site_admin",
"starred_url",
"subscriptions_url",
"type",
"url"
]
},
"assets": {
"type": "array",
"items": {
"title": "Release Asset",
"description": "Data related to a release.",
"type": "object",
"properties": {
"url": {
"type": "string",
"format": "uri"
},
"browser_download_url": {
"type": "string",
"format": "uri"
},
"id": {
"type": "integer"
},
"node_id": {
"type": "string"
},
"name": {
"description": "The file name of the asset.",
"type": "string"
},
"label": {
"type": [
"string",
"null"
]
},
"state": {
"description": "State of the release asset.",
"type": "string",
"enum": [
"uploaded",
"open"
]
},
"content_type": {
"type": "string"
},
"size": {
"type": "integer"
},
"digest": {
"type": [
"string",
"null"
]
},
"download_count": {
"type": "integer"
},
"created_at": {
"type": "string",
"format": "date-time"
},
"updated_at": {
"type": "string",
"format": "date-time"
},
"uploader": {
"anyOf": [
{
"type": "null"
},
{
"title": "Simple User",
"description": "A GitHub user.",
"type": "object",
"properties": {
"name": {
"type": [
"string",
"null"
]
},
"email": {
"type": [
"string",
"null"
]
},
"login": {
"type": "string"
},
"id": {
"type": "integer",
"format": "int64"
},
"node_id": {
"type": "string"
},
"avatar_url": {
"type": "string",
"format": "uri"
},
"gravatar_id": {
"type": [
"string",
"null"
]
},
"url": {
"type": "string",
"format": "uri"
},
"html_url": {
"type": "string",
"format": "uri"
},
"followers_url": {
"type": "string",
"format": "uri"
},
"following_url": {
"type": "string"
},
"gists_url": {
"type": "string"
},
"starred_url": {
"type": "string"
},
"subscriptions_url": {
"type": "string",
"format": "uri"
},
"organizations_url": {
"type": "string",
"format": "uri"
},
"repos_url": {
"type": "string",
"format": "uri"
},
"events_url": {
"type": "string"
},
"received_events_url": {
"type": "string",
"format": "uri"
},
"type": {
"type": "string"
},
"site_admin": {
"type": "boolean"
},
"starred_at": {
"type": "string"
},
"user_view_type": {
"type": "string"
}
},
"required": [
"avatar_url",
"events_url",
"followers_url",
"following_url",
"gists_url",
"gravatar_id",
"html_url",
"id",
"node_id",
"login",
"organizations_url",
"received_events_url",
"repos_url",
"site_admin",
"starred_url",
"subscriptions_url",
"type",
"url"
]
}
]
}
},
"required": [
"id",
"name",
"content_type",
"size",
"digest",
"state",
"url",
"node_id",
"download_count",
"label",
"uploader",
"browser_download_url",
"created_at",
"updated_at"
]
}
},
"body_html": {
"type": "string"
},
"body_text": {
"type": "string"
},
"mentions_count": {
"type": "integer"
},
"discussion_url": {
"description": "The URL of the release discussion.",
"type": "string",
"format": "uri"
},
"reactions": {
"title": "Reaction Rollup",
"type": "object",
"properties": {
"url": {
"type": "string",
"format": "uri"
},
"total_count": {
"type": "integer"
},
"+1": {
"type": "integer"
},
"-1": {
"type": "integer"
},
"laugh": {
"type": "integer"
},
"confused": {
"type": "integer"
},
"heart": {
"type": "integer"
},
"hooray": {
"type": "integer"
},
"eyes": {
"type": "integer"
},
"rocket": {
"type": "integer"
}
},
"required": [
"url",
"total_count",
"+1",
"-1",
"laugh",
"confused",
"heart",
"hooray",
"eyes",
"rocket"
]
}
},
"required": [
"assets_url",
"upload_url",
"tarball_url",
"zipball_url",
"created_at",
"published_at",
"draft",
"id",
"node_id",
"author",
"html_url",
"name",
"prerelease",
"tag_name",
"target_commitish",
"assets",
"url"
]
}There was a problem hiding this comment.
Done in 2a21ca7 — RepositoryRelease now matches the Release response schema. I removed the input-only fields (make_latest, discussion_category_name, generate_release_notes), which are carried by CreateReleaseRequest/UpdateReleaseRequest and aren't part of the Release response, and added the response fields that were missing: updated_at, body_html, body_text, mentions_count, discussion_url and reactions. Accessors and stringify tests are regenerated. Thanks!
| TagName *string `json:"tag_name,omitempty"` | ||
| TargetCommitish *string `json:"target_commitish,omitempty"` | ||
| Name *string `json:"name,omitempty"` | ||
| Body *string `json:"body,omitempty"` | ||
| Draft *bool `json:"draft,omitempty"` | ||
| Prerelease *bool `json:"prerelease,omitempty"` | ||
| Immutable *bool `json:"immutable,omitempty"` | ||
| ID *int64 `json:"id,omitempty"` | ||
| CreatedAt *Timestamp `json:"created_at,omitempty"` | ||
| PublishedAt *Timestamp `json:"published_at,omitempty"` | ||
| UpdatedAt *Timestamp `json:"updated_at,omitempty"` | ||
| URL *string `json:"url,omitempty"` | ||
| HTMLURL *string `json:"html_url,omitempty"` | ||
| AssetsURL *string `json:"assets_url,omitempty"` | ||
| Assets []*ReleaseAsset `json:"assets,omitempty"` | ||
| UploadURL *string `json:"upload_url,omitempty"` | ||
| ZipballURL *string `json:"zipball_url,omitempty"` | ||
| TarballURL *string `json:"tarball_url,omitempty"` | ||
| Author *User `json:"author,omitempty"` | ||
| NodeID *string `json:"node_id,omitempty"` | ||
| BodyHTML *string `json:"body_html,omitempty"` | ||
| BodyText *string `json:"body_text,omitempty"` | ||
| MentionsCount *int `json:"mentions_count,omitempty"` | ||
| DiscussionURL *string `json:"discussion_url,omitempty"` | ||
| Reactions *Reactions `json:"reactions,omitempty"` |
There was a problem hiding this comment.
These fields can be without omitempty:
"required": [
"assets_url",
"upload_url",
"tarball_url",
"zipball_url",
"created_at",
"published_at",
"draft",
"id",
"node_id",
"author",
"html_url",
"name",
"prerelease",
"tag_name",
"target_commitish",
"assets",
"url"
]
There was a problem hiding this comment.
Done — dropped omitempty from the fields in the schema's required list (they're always present in a release response). The remaining optional fields (body, immutable, updated_at, body_html, body_text, mentions_count, discussion_url, reactions) keep it. Thanks!
2a21ca7 to
13f0fb7
Compare
| Author *User `json:"author,omitempty"` | ||
| NodeID *string `json:"node_id,omitempty"` | ||
| Immutable *bool `json:"immutable,omitempty"` | ||
| TagName *string `json:"tag_name"` |
There was a problem hiding this comment.
| TagName *string `json:"tag_name"` | |
| TagName string `json:"tag_name"` |
@gmlewis maybe we need to have a linter to detect this: *string cannot be with json:"tag_name" (without omitempty).
There was a problem hiding this comment.
Done — converted the required, non-nullable scalar fields to value types: TagName, TargetCommitish, ID, CreatedAt, URL, HTMLURL, AssetsURL, UploadURL, NodeID.
A few required fields stay pointers, on purpose:
Draft/Prerelease— go-github has no valueboolfields; the stringify test generator (gen-stringify-test.go) doesn't handle a valueboolzero value, so these remain*bool.Name,PublishedAt,ZipballURL,TarballURL— these are nullable (["string", "null"]) in the schema, so they need a pointer to representnull.Author— nested objects use pointers by convention (e.g.Codespace.BillableOwner *User \json:"billable_owner"``).
Let me know if you'd prefer a different split.
There was a problem hiding this comment.
the stringify test generator (gen-stringify-test.go) doesn't handle a value bool zero value, so these remain
So, maybe we can update the generator to handle a bool?
These two required fields should be non-pointers:
Draft bool `json:"draft"`
Prerelease bool `json:"prerelease"`
There was a problem hiding this comment.
Done in a24cfe7 — I updated the generator so a value bool works, then made both fields non-pointers.
gen-stringify-test.go:processZeroValuenow has acase "false"(mirroring the existingPtr(false)case), so a valueboolzero value renders correctly. This was the only gap —addIdentalready emitted"false"for a valuebool; the helper just had no matching case and wouldlog.Fatalf.RepositoryRelease:DraftandPrereleaseare nowbool(json:"draft"/json:"prerelease"), since both are required, non-nullable booleans in the Release schema.- Regenerated the accessors and stringify test.
GetDraft/GetPrereleasekeep theirboolsignatures, so callers are unaffected.
I kept CreateReleaseRequest/UpdateReleaseRequest as *bool,omitempty, since draft/prerelease are optional inputs on the create/update endpoints. Thanks!
…Release Introduce dedicated CreateReleaseRequest and UpdateReleaseRequest types for the bodies of RepositoriesService.CreateRelease and RepositoriesService.UpdateRelease, replacing the *RepositoryRelease parameter that also carried response-only fields. The new types are passed by value and serialized directly, dropping the internal repositoryReleaseRequest remap. EditRelease is renamed to UpdateRelease for naming consistency. This follows the value-parameter pattern established by the merged google#3654, google#3794 and google#4320, the Edit -> Update rename from google#4320, and the dedicated *Request body convention already used across the package (e.g. CreateHostedRunnerRequest). The runtime nil checks are removed since a value parameter makes them unnecessary. No deprecated wrappers are added (clean break). Updates google#3644.
Remove the input-only fields MakeLatest, DiscussionCategoryName and GenerateReleaseNotes from RepositoryRelease — these are carried by CreateReleaseRequest/UpdateReleaseRequest and are not part of the Release response schema. Add the response fields that were missing compared to the schema: UpdatedAt, BodyHTML, BodyText, MentionsCount, DiscussionURL and Reactions. Regenerate the accessors and stringify tests accordingly.
67211d5 to
8e85f6f
Compare
Convert the required, non-nullable scalar fields of RepositoryRelease to value types: TagName, TargetCommitish, Draft, Prerelease, ID, CreatedAt, URL, HTMLURL, AssetsURL, UploadURL and NodeID. Draft and Prerelease are required, non-nullable booleans in the Release schema, so they become value bool. To support a value bool zero value, teach gen-stringify-test.go's processZeroValue to render it (previously only the *bool form was handled). The nullable required fields (Name, PublishedAt, ZipballURL, TarballURL) stay pointers so they can represent null, and Author stays *User per the nested-object convention. Regenerate the accessors and stringify tests.
8e85f6f to
a24cfe7
Compare
Survey of open issues and PRs in google/go-github with root-cause analysis, resolution approach, difficulty, and disposition for each. Tracks the fork owner's submitted PRs and issue triage outcomes (through google#4329 merge and google#2898 close, 2026-06-26).
Survey of open issues and PRs in google/go-github with root-cause analysis, resolution approach, difficulty, and disposition for each. Tracks the fork owner's submitted PRs and issue triage outcomes (through google#4329 merge and google#2898 close, 2026-06-26).
…ditReleaseAsset` to `UpdateReleaseAsset` Continues the google#3644 value-parameter migration for repos_releases.go, finishing the file after CreateRelease/UpdateRelease (google#4329). - GenerateReleaseNotes now takes GenerateNotesRequest by value (renamed from GenerateNotesOptions). - EditReleaseAsset is renamed to UpdateReleaseAsset and takes a dedicated UpdateReleaseAssetRequest{Name, Label} by value instead of reusing the *ReleaseAsset response struct. - Drop the now-converted types (GenerateNotesOptions, ReleaseAsset) from the paramcheck allowlists in .golangci.yml. Generated accessors regenerated accordingly. Updates google#3644
Survey of open issues and PRs in google/go-github with root-cause analysis, resolution approach, difficulty, and disposition for each. Tracks the fork owner's submitted PRs and issue triage outcomes (through google#4329 merge and google#2898 close, 2026-06-26).
Survey of open issues and PRs in google/go-github with root-cause analysis, resolution approach, difficulty, and disposition for each. Tracks the fork owner's submitted PRs and issue triage outcomes (through google#4329 merge and google#2898 close, 2026-06-26).
Survey of open issues and PRs in google/go-github with root-cause analysis, resolution approach, difficulty, and disposition for each. Tracks the fork owner's submitted PRs and issue triage outcomes (through google#4329 merge and google#2898 close, 2026-06-26).
Survey of open issues and PRs in google/go-github with root-cause analysis, resolution approach, difficulty, and disposition for each. Tracks the fork owner's submitted PRs and issue triage outcomes (through google#4329 merge and google#2898 close, 2026-06-26).

BREAKING CHANGE:
CreateRelease&UpdateReleasenow takeRepositoryReleaseby value;EditReleaseis renamed toUpdateRelease.Towards #3644.
Passes the request body of
RepositoriesService.CreateReleaseandRepositoriesService.UpdateReleaseby value instead of by pointer, and renamesEditReleasetoUpdateReleasefor naming consistency.This continues the value-parameter conversion from the merged #3654, #3794 and #4320, and follows the
Edit->Updaterename done forGistsServicein #4320.Changes
CreateRelease(ctx, owner, repo string, release *RepositoryRelease)->release RepositoryReleaseEditRelease(ctx, owner, repo string, id int64, release *RepositoryRelease)->UpdateRelease(..., release RepositoryRelease)release == nilchecks, which the value signature now makes unnecessary.GistsServicerequired params by value #4320.Breaking changes
CreateRelease/UpdateReleasenow takeRepositoryReleaseby value.EditReleaseis renamed toUpdateRelease.The remaining body-pointer methods in
repos_releases.go(EditReleaseAsset,GenerateReleaseNotes) are intentionally left for a follow-up, since they also involve allow-list and type-name changes.Generated files are unchanged (no struct fields changed).