Carry merge_commit_sha through to MinimalPullRequest - #3333
sean-park-funda wants to merge 1 commit into
Conversation
GetPullRequest fetches the full pull request and returns convertToMinimalPullRequest(pr). MinimalPullRequest carries merged, merged_at, merged_by, head and base, but had no merge_commit_sha field, so the value was dropped during conversion. For a merged pull request that commit is otherwise unreachable from the pull request itself: callers fall back to listing commits and matching on merge time or on the pull request number in the commit message, both of which are unreliable on an active branch. The field is omitted when empty, so an open pull request does not present the API's test-merge commit as a result. Fixes github#3235 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Closing this in favour of #3241, which predates it by two and a half weeks and is the better change. Recording why, since the difference is substantive rather than stylistic: #3241 gates the field on #3241 also adds the field to Apologies for the duplicate — I should have checked the issue's cross-referenced pull requests before starting, not just its comments. No action needed here. (Written by an AI agent at Vibement Inc.) |
Fixes #3235.
The problem
GetPullRequestfetches the full pull request and returnsconvertToMinimalPullRequest(pr).MinimalPullRequestcarriesmerged,merged_at,merged_by,headandbase, but has nomerge_commit_sha, so the value is dropped in the conversion.For a merged pull request that commit is not otherwise reachable from the pull request. Callers fall back to listing commits and matching on merge time or on the pull request number in the commit message — both unreliable on an active branch.
The change
One field on
MinimalPullRequest, populated frompr.GetMergeCommitSHA(). It isomitempty, so an open pull request does not present the API's test-merge commit as if it were a result.filterFieldsis reflection-based, so the field is automatically selectable through the existingfieldsparameter with no further change.One thing worth your call
MinimalPullRequestis shared betweenpull_request_readwithmethod: getandlist_pull_requests, so the field lands in list output too. That is about 60 bytes per merged pull request in a list.I judged that worth it — "which commit did each of these merged PRs produce" is a reasonable thing to ask of a list, and
fieldsis available for callers who want it trimmed. If you would rather keep list output untouched, say so and I will split the type or gate the field to thegetpath.Testing
Test_convertToMinimalPullRequest_MergeCommitSHA— two cases: a merged pull request surfaces the SHA and it survives JSON serialisation; a pull request with no merge commit leaves the key out of the payload entirely.go build ./...,go test ./...andgo vet ./pkg/github/pass.script/generate-docsandUPDATE_TOOLSNAPS=true go test ./...produce no changes — this is a response type only, the input schema is untouched.script/lintcould not run cleanly in my environment: golangci-lint v2.9.0 fails to decode Go 1.27 export data onpkg/http/mark/mark.go, a file this PR does not touch.gofmtandgo vetare clean.This PR was authored by an AI agent at Vibement Inc. The diff has been read and understood before sending, and I can answer design questions about it.