Repository navigation
fix(ci): check protobuf compatibility against the pull request's merge base - #1781
Merged
Merged
Conversation
…e base The breaking-change step diffed api/proto against the base branch's tip, so a field another pull request added after a branch was cut failed that branch as a deleted field until it merged the base. The baseline is now the merge base with the base branch (a merge group's base_sha), reached by a blob-free history fetch; a hygiene test pins it. Co-Authored-By: jason.han <hanhuijun@gmail.com>
Contributor
Author
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-Authored-By: jason.han <hanhuijun@gmail.com>
4 of 6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
The pull-request workflow's
Check the protobuf schema for breaking changesstep ranmake proto-breakingagainstorigin/<base branch>— the base branch's tip at run time. A field another pull request adds toapi/proto/sysml.protoafter a branch was cut therefore read as a deleted field on that branch, and itsGo static and integrity checks(and theBuild and testaggregate over it) went red until the branch merged the base, with nothing wrong on either side. #1561 hit this when #1558 addedinheritedtoRenderNode/RenderPort.The baseline is now the schema the pull request started from:
git merge-base origin/<base> <pull_request.head.sha>for a pull request, and the merge group's ownbase_shafor a queue commit (the merge-group payload already names it). The shallow checkout gets the history formerge-basefrom a blob-free fetch (--filter=tree:0 --unshallow; the archive the Makefile target takes lazily fetches the one subtree it needs), so the job fetches no more content than before.docs/project/releasing.mdsays the same where it describes the release PR's comparison; the Makefile target and itsorigin/developdefault are unchanged.Every read of the pull-request payload keeps its
merge_groupbranch, asTestPRWorkflowPullRequestReadsHaveMergeGroupBranchrequires; the newTestPRWorkflowProtoBaselineIsTheMergeBasepins the baseline to the merge base /base_shaso the step cannot drift back to the branch tip.How it was verified
go test ./tests/hygiene -run 'TestPRWorkflow|TestRaceShard|TestMergeGroup'ok (new test included);go vet,gofmt -lclean;python3 scripts/changelog.py checkok.actionlint .github/workflows/pr.ymlandshellcheck -s bashon the step body: no findings on the changed step (the two actionlint style notes in the file are pre-existing, on other steps).--depth 1clone ofdevelopagainst GitHub withBASE_REF=refs/heads/develop HEAD_SHA=<a pull request head>: the blob-free fetch takes about 2 s and 36 MB,merge-baseresolves,git archiveof<baseline>:api/protoworks on the partial history,make proto-breakingreports✓ No breaking schema changes.Checklist
make testandmake lintpass locally (the affected package,tests/hygiene; no Go production code changed)changes/unreleased/<slug>.<section>.md, not as an edit toCHANGELOG.mdmake docs-countsrun if a gate count moved (compliance rows need nothing: the census is counted at docs build) — none movedF4,K5) in the body, docs, or changelogLink to Devin session: https://nasa-jpl-demo.devinenterprise.com/sessions/4b77801c1b7a447cb2c91401b67b47d7
Open in Devin Desktop: https://nasa-jpl-demo.devinenterprise.com/desktop/session/4b77801c1b7a447cb2c91401b67b47d7?variant=devin
Requested by: @HuiJun