refactor!: Pass `GistsService` required params by value by JamBalaya56562 · Pull Request #4320 · google/go-github

JamBalaya56562 · 2026-06-23T00:29:04Z

BREAKING CHANGE: GistsService methods now pass required params by-value instead of by-ref.

What problem are you solving?

This completes #3680 (open and unanswered for ~1 year), which addresses #3644 by
passing required GistsService inputs by value instead of by pointer.

It applies the review feedback that was agreed on #3680 but never implemented by
the original author:

Introduce dedicated value-type request structs: CreateGistRequest,
UpdateGistRequest, CreateGistCommentRequest, UpdateGistCommentRequest.
Create and CreateComment now take the request struct by value.
Rename Edit → Update and EditComment → UpdateComment for API
consistency (suggested by @stevehipwell, agreed by @gmlewis).
No deprecated wrappers (per @alexandear and @gmlewis on refactor!: Refactor GistsService to use value parameters #3680; consistent
with the earlier GitService refactor in refactor!: Change GitService methods to pass required params by-value instead of by-ref #3654).
Remove the now-obsolete Gist / GistComment entries from the paramcheck
body-allowed-pointer-types allow-list in .golangci.yml.

BREAKING CHANGE: the Create / Update / CreateComment / UpdateComment
signatures change, and Edit / EditComment are renamed.

Patch coverage is 100% (the four methods and the generated accessors for the new
request structs are fully covered); generated files are regenerated and the
OpenAPI metadata is unchanged (the method renames do not affect //meta:operation).

~~Relates to~~ Fixes: #3644.
~~Supersedes~~ Closes: #3680.

Note: this PR was prepared with AI assistance (Claude Code); all changes were
reviewed and verified locally (build, full github test suite, custom
golangci-lint, and gofmt).

google-cla · 2026-06-23T00:29:21Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

BREAKING CHANGE: GistsService.Create / Update / CreateComment / UpdateComment now take dedicated request structs by value (CreateGistRequest, UpdateGistRequest, CreateGistCommentRequest, UpdateGistCommentRequest) instead of *Gist / *GistComment. Edit is renamed to Update and EditComment is renamed to UpdateComment. This completes the stalled PR google#3680, applying the maintainer-agreed review feedback that was never addressed: drop the deprecated wrappers entirely and rename Edit/EditComment to Update/UpdateComment for API consistency. The now-obsolete Gist and GistComment entries are removed from the paramcheck body-allowed-pointer-types allow-list in .golangci.yml. Relates to google#3644.

codecov · 2026-06-23T00:38:51Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.48%. Comparing base (728cdb1) to head (b85f702).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4320   +/-   ##
=======================================
  Coverage   97.48%   97.48%           
=======================================
  Files         193      193           
  Lines       19400    19400           
=======================================
  Hits        18912    18912           
  Misses        270      270           
  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, @JamBalaya56562!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.

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

alexandear · 2026-06-23T10:01:02Z

+type CreateGistRequest struct {
+	Description *string                   `json:"description,omitempty"`
+	Public      *bool                     `json:"public,omitempty"`
+	Files       map[GistFilename]GistFile `json:"files,omitempty"`


This parameter is required:

Suggested change

Files map[GistFilename]GistFile `json:"files,omitempty"`

Files map[GistFilename]GistFile `json:"files"`

Done — dropped omitempty so files is always sent (json:"files"), and it now uses GistFileRequest. b85f702

alexandear · 2026-06-23T10:03:20Z

+// UpdateGistRequest represents the input for updating a gist.
+type UpdateGistRequest struct {
+	Description *string                   `json:"description,omitempty"`
+	Files       map[GistFilename]GistFile `json:"files,omitempty"`


Please add a description comment for the field:

Done — added doc comments to the Files field and the new GistFileRequest. b85f702

alexandear · 2026-06-23T10:07:46Z

+type CreateGistRequest struct {
+	Description *string                   `json:"description,omitempty"`
+	Public      *bool                     `json:"public,omitempty"`
+	Files       map[GistFilename]GistFile `json:"files,omitempty"`


map[GistFilename]GistFile != files object from the schema

See

Please introduce a new structure.

Done — introduced GistFileRequest{Content, Filename} for the request file value instead of reusing the GistFile response type. Confirmed against the OpenAPI: the create file value is {content} (content required) and update is {content, filename}, so neither matches the read-only-heavy GistFile. b85f702

I'm sorry that I missed this before, but I think the maps should be maps to pointers, since the values are structs. This would match our preference to have slices of pointers to structs.

@alexandear - do you agree?

Agree.

@JamBalaya56562 Please use a pointer for values in the map.

Good point — switched the Files maps to pointer values (map[GistFilename]*CreateGistFile / *UpdateGistFile) to match the pointers-to-structs preference. a332585

Per review, the gist create/update request files map should not reuse the GistFile response type (which carries read-only fields like size, raw_url, type and language). Introduce GistFileRequest{Content, Filename} for the request file value, mark CreateGistRequest.Files required (drop omitempty), and add field doc comments. The OpenAPI schema confirms the create file value is {content} (content required) and the update file value is {content, filename}.

alexandear · 2026-06-23T11:39:57Z

+// GistFileRequest represents a file's content within a CreateGistRequest or
+// UpdateGistRequest. The gist's files are keyed by filename.
+type GistFileRequest struct {
+	// Content is the contents of the file. (Required when creating a gist.)
+	Content *string `json:"content,omitempty"`
+	// Filename, if set, renames the file. (Used when updating a gist.)
+	Filename *string `json:"filename,omitempty"`
+}


It's better to have two separate structs for creating and for updating.

Done — split into separate CreateGistFile ({content}) and UpdateGistFile ({content, filename}) structs. a332585

alexandear · 2026-06-23T11:44:04Z

+	// Files is the set of files to add, change or rename, keyed by filename.
+	Files map[GistFilename]GistFileRequest `json:"files,omitempty"`


We can also delete a file. See

The comment is not full.

Good catch — UpdateGistFile now documents that mapping a filename to a nil value deletes the file, and I added a test for it. a332585

Per review, use separate CreateGistFile and UpdateGistFile structs for the request files (matching the distinct create/update schemas), and key the Files maps to pointers (map[GistFilename]*...), matching the repo's preference for pointers to structs. A nil value in an UpdateGistRequest's Files map deletes that file from the gist, which is now documented and covered by a test.

gmlewis changed the title ~~refactor!: Pass GistsService required params by value~~ refactor!: Pass GistsService required params by value Jun 23, 2026

JamBalaya56562 force-pushed the gists-value-params branch from 993c86d to 0a16bd5 Compare June 23, 2026 00:30

JamBalaya56562 force-pushed the gists-value-params branch from 0a16bd5 to 2a65d2f Compare June 23, 2026 00:33

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 Jun 23, 2026

gmlewis approved these changes Jun 23, 2026

View reviewed changes

alexandear suggested changes Jun 23, 2026

View reviewed changes

alexandear reviewed Jun 23, 2026

View reviewed changes

JamBalaya56562 requested review from alexandear and gmlewis June 23, 2026 11:12

alexandear suggested changes Jun 23, 2026

View reviewed changes

JamBalaya56562 requested a review from alexandear June 23, 2026 12:59

	Files map[GistFilename]GistFile `json:"files,omitempty"`
	Files map[GistFilename]GistFile `json:"files"`

		// Files is the set of files to add, change or rename, keyed by filename.
		Files map[GistFilename]GistFileRequest `json:"files,omitempty"`

Conversation

JamBalaya56562 commented Jun 23, 2026 • edited by gmlewis Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem are you solving?

Uh oh!

google-cla Bot commented Jun 23, 2026

Uh oh!

codecov Bot commented Jun 23, 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!

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!

alexandear Jun 23, 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!

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!

3 participants

JamBalaya56562 commented Jun 23, 2026 •

edited by gmlewis

Loading

codecov Bot commented Jun 23, 2026 •

edited

Loading

alexandear Jun 23, 2026 •

edited

Loading