refactor!: Rename PullRequestReviewDismissalRequest to PullRequestDismissReviewRequest, add PullRequestSubmitReviewRequest, and pass review request bodies by value#4406
Conversation
message is required by the dismiss-a-review endpoint, so `Message` becomes a non-pointer string without omitempty, and `PullRequestsService.DismissReview` now takes the request body by value. The type is removed from the .golangci.yml allowlist. BREAKING CHANGE: PullRequestReviewDismissalRequest.Message is now a non-pointer string, and PullRequestsService.DismissReview now takes the request body by value.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4406 +/- ##
=======================================
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. 🚀 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.
Let's also fix the body in PullRequestsService.SubmitReview.
…DismissReviewRequest` Aligns the request type name with the PullRequestsService.DismissReview method name, as suggested in review.
The dismiss-a-review endpoint accepts an optional event parameter (can be "DISMISS") that was missing from the Go struct.
…uestSubmitReviewRequest` SubmitReview reused *PullRequestReviewRequest, whose node_id, commit_id and comments fields are not part of the submit-a-review request schema. The new PullRequestSubmitReviewRequest models that schema exactly: event is required and becomes a non-pointer string, body stays optional. CreateReview keeps the shared PullRequestReviewRequest, so its allowlist entry remains.
PullRequestReviewDismissalRequest by valuePullRequestReviewDismissalRequest to PullRequestDismissReviewRequest, add PullRequestSubmitReviewRequest, and pass review request bodies by value
Done in 8383de1. Rather than passing the shared PR title and description updated to cover all three changes. |
Continues the request-body-by-value work in #3644, for the review dismissal and submission endpoints on
PullRequestsService.Dismiss a review
messageis required, soMessagebecomes a non-pointerstring(noomitempty), andDismissReviewtakes the body by value. The type is removed from thebody-allowed-pointer-typesallowlist.PullRequestDismissReviewRequestto match the method name, and the previously unmodeled optionaleventparameter is added asEvent *string.Submit a review
Per review,
SubmitReviewno longer reuses*PullRequestReviewRequest— whosenode_id,commit_idandcommentsfields aren't part of the submit-a-review schema — and instead takes a newPullRequestSubmitReviewRequestby value, modeling that schema exactly:Eventis required (non-pointerstring),Bodyis optional (*string).CreateReviewkeeps the sharedPullRequestReviewRequest, so its allowlist entry remains.Verified with
go build ./...,go vet -tags integration ./test/integration/,gofmt, the full./github/test suite, andcustom-gcl(noparamcheckfindings).Updates #3644
BREAKING CHANGE: PullRequestReviewDismissalRequest is renamed to PullRequestDismissReviewRequest with non-pointer Message; PullRequestsService.DismissReview takes it by value; PullRequestsService.SubmitReview now takes a new PullRequestSubmitReviewRequest (non-pointer Event) by value instead of *PullRequestReviewRequest.
cc @jvm986 — flagging for #3644 coordination; this is in the
pullsservice, so it shouldn't overlap with theIssueswork.