Skip to content

refactor!: Split RepositoryComment request bodies and pass by value - #4496

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

refactor!: Split RepositoryComment request bodies and pass by value#4496
JamBalaya56562 wants to merge 2 commits into
google:masterfrom
JamBalaya56562:refactor/3644-commit-comment-value-params

Conversation

@JamBalaya56562

Copy link
Copy Markdown
Contributor

Continues the request-body-by-value work in #3644 — the commit-comments counterpart of #4444 (IssueComment, issues) and #4493 (PullRequestComment, pulls), completing the comment-endpoint family.

Commit 1 (refactor!:)RepositoriesService.CreateComment and UpdateComment reused the 12-field RepositoryComment response type as their request bodies. Per the create / update docs the two request schemas differ, so this splits them into dedicated types (same shape as #4493):

body path position line
create required optional optional optional (deprecated — "use position instead")
update required
type CreateCommitCommentRequest struct {
	Body     string  `json:"body"`
	Path     *string `json:"path,omitempty"`
	Position *int    `json:"position,omitempty"`
	// Deprecated: Use Position instead.
	Line *int `json:"line,omitempty"`
}

type UpdateCommitCommentRequest struct {
	Body string `json:"body"`
}

Both methods now take these by value (Body required → non-pointer, no omitempty), and RepositoryComment is removed from the paramcheck allowlist in .golangci.yml. No method renames are needed — Create/Update already match the docs operation names. Alongside:

  • the now-stale // User-mutable fields / // User-initialized fields markers on RepositoryComment are dropped and Body's tag normalized to json:"body,omitempty", making it a plain response type like IssueComment / PullRequestComment;
  • the {comment_id} parameter is named commentID consistently across GetComment / UpdateComment / DeleteComment (matching the issues/pulls comment files);
  • the request types also gain the deprecated line create parameter, which the old shared type could not send at all.

Commit 2 (feat:, non-breaking) — cross-checking the full commit-comment response schema against the struct turned up two missing fields, added here with regenerated accessors: AuthorAssociation and Line.

BREAKING CHANGE: RepositoriesService.CreateComment now takes CreateCommitCommentRequest and RepositoriesService.UpdateComment takes UpdateCommitCommentRequest, both passed by value instead of *RepositoryComment.

Verified with go build ./..., gofmt, go vet -tags integration ./test/integration/, the full ./github/ test suite (all repos_comments.go methods at 100% coverage), generator idempotency, and custom-gcl (0 issues with the allowlist entry removed).

Updates #3644.

cc @jvm986

RepositoriesService.CreateComment and UpdateComment reused the
12-field RepositoryComment response type as their request bodies. The
two endpoints accept different fields (create: body, path, position,
line; update: body only), so introduce dedicated
CreateCommitCommentRequest and UpdateCommitCommentRequest types, pass
them by value, and drop RepositoryComment from the paramcheck
allowlist. Also align the {comment_id} parameter name to commentID
across GetComment, UpdateComment, and DeleteComment.

BREAKING CHANGE: `RepositoriesService.CreateComment` now takes `CreateCommitCommentRequest` and `RepositoriesService.UpdateComment` takes `UpdateCommitCommentRequest`, both passed by value instead of `*RepositoryComment`.

Updates google#3644.
The commit-comment response schema includes author_association and
line, which were missing from the struct.
@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 Aug 29, 2026
@codecov

codecov Bot commented Aug 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.52%. Comparing base (95224ef) to head (b85f7af).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4496   +/-   ##
=======================================
  Coverage   98.52%   98.52%           
=======================================
  Files         195      195           
  Lines       17745    17745           
=======================================
  Hits        17484    17484           
  Misses        261      261           

☔ 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 - @alexandear - @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