Repository navigation
🏗️🔧:fix four faults in landing a pull request - #914
Conversation
Porting this machinery into OpenINF/sdk put it under review again, and four things came out of that. All of them are here as well, which is where they were written, so they are fixed here first. A branch of more than thirty commits could not land at all. The commits are asked of the API with `--paginate` and a `--jq` filter, and those two together filter each page and print the results one after another, so anything past the first page handed `JSON.parse` several arrays in a row. What came back was a parse error rather than a reason. `--slurp` gathers the pages into one array instead, and the filtering that `--jq` was doing happens in the task. The 72-column limit was applied to trailers, which cannot be wrapped to meet it: git would read a folded value, but `readTrailers` keeps only the token line, so folding a long `Signed-off-by:` moves the author out of reach of `checkSignOff` and fails a different way. Anyone whose name and address ran past 57 characters could not write a commit this would accept. The trailer block is exempt now; prose is not, including a last paragraph that only looks like trailers and so holds no trailers at all. A folded trailer was split while a landing message was composed. Each line was read on its own, so the token line went to the trailers and the indented continuation stayed in the body: `Co-authored-by:` written over two lines landed with the address left behind. The message still validated, so the queue merged it and the attribution was quietly lost. A continuation is attached to the trailer above it now, rather than pushed as an entry of its own, which would have let the sort move it away from what it belongs to. The pull request template told contributors to paste the emoji into the description. It belongs in the title, which is the subject that lands and the only place anything reads it. Signed-off-by: Derek Lewis <DerekNonGeneric@inf.is> Assisted-by: Claude-Code:claude-opus-5
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe changes update commit trailer validation and folded trailer parsing, fix paginated commit retrieval, add related tests, and correct the pull request template instruction for emoji placement. ChangesCommit Message Handling
Paginated Commit Retrieval
Pull Request Guidance
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The landing workflow fixes are covered by regression tests, with no actionable merge-blocking risk identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 markdownlint-cli2 (0.23.2).github/PULL_REQUEST_TEMPLATE.mdmarkdownlint-cli2 v0.23.2 (markdownlint v0.41.1) ... [truncated 1052 characters] ... Resolution (node:internal/modules/esm/resolve:271:11) Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Porting this machinery into OpenINF/sdk put it under review again, and four defects came out of that. They were written here, so they are fixed here. Each was reproduced before being fixed, and each has regression cases.
A branch of more than thirty commits could not land at all.
land-pull-request.mtsasks the API for the branch's commits withgh api --paginateand a--jqfilter. Those two together filter each page and print the results one after another, so anything past the first page handsJSON.parseseveral arrays in a row:JSON.parsethen fails withUnexpected non-whitespace character after JSON at position 88, the outer catch reports a parse error rather than a reason, and nothing lands. It now uses--paginate --slurpwithout--jq, which gathers the pages into one array, and does the merge-dropping filter in the task.A long enough sign-off was impossible to write. The 72-column limit was applied to every line after the subject, trailers included.
Signed-off-by: Christopher Alexander Montgomery <christopher.montgomery@example.org>is 84 characters and was rejected as a body-width error. Folding it is no escape, becausereadTrailerskeeps only the token line, so the address moves out of reach ofcheckSignOffand that fails instead. Anyone whose name and address run past 57 characters was stuck between the two. The trailer block is now exempt from the limit. Prose is not, including a last paragraph that only looks like trailers and so holds none.A folded trailer lost half of itself when landing.
partsOfMessageread each line on its own, so the token line went to the trailers while an indented continuation stayed in the body.Co-authored-by:written over two lines landed with the address left behind, and the composed message still validated, so the queue merged it and the attribution was quietly lost. Continuations now attach to the trailer above them rather than being pushed as entries of their own, which would have letranksort them away from what they belong to.The pull request template pointed at the wrong field. It said to paste the emoji into the description; it belongs in the title, which is the subject that lands and the only place anything reads it. Following the template as written produced a valid description and an invalid title.
Validation
nps test— every verify task passes, includingverify.commitson this commitparentsincluded so the merge filter still worksNote
The portal carries the same four, and the same fix, on its own
infra/commit-queue-defects.OpenINF/sdkalready has them, since porting this there is where review found them.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Documentation