Repository navigation
feat(pr-validation): mark the breaking change so it reads at a glance - #909
Merged
Merged
Conversation
The alert block said what happened in prose, which meant reading it to know. A 💥 in an `##` heading inside the block makes the state legible from the timeline, before anyone opens anything — and the table gains marks on the three rows that matter, so it can be scanned instead of read. Each state now gets the alert that fits it: IMPORTANT while a decision is pending, NOTE once approved, CAUTION on a rejection that closed the pull request, WARNING when the decision was never recorded. Colour and icon come from GitHub; the emoji says what the block is about.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe breaking-change approval reporter now uses state-specific GitHub alerts. The waiting-state table updates its commit and author-acknowledgement labels. A test checks the alert types and state headings. ChangesBreaking-change approval alerts
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Feature Suggested reviewers: Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 1 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly summarizes the main change: making breaking-change status visible at a glance. It is concise and specific. Description check The description explains the presentation changes, identifies the affected behavior, marks the change type, states that there are no breaking API changes, and documents testing. It also clearly notes … Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Usage-based review receipt
- Mode: Continue automatically
- Reviewed files: 1
- Charged: $0.25
- View usage details
Note
This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.
Comment @coderabbitai help to get the list of available commands.
Contributor
🔍 Lint Analysis
|
lerian-studio
added
size/S
Contributor
🔍 PR Validation Summary✅ PR Mergeable — no blocking failures
|
Contributor
🛡️ CodeQL Analysis ResultsLanguages analyzed: ✅ No security issues found. 🔍 View full scan logs | 🛡️ Security tab |
This comment has been minimized.
This comment has been minimized.
lerian-studio
added
the
review-ready
This comment has been minimized.
This comment has been minimized.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Addresses the CodeRabbit review on #909. The state assertions searched the whole reporter script, so every alert and heading was present somewhere no matter which branch used it. Swapping two states' alerts would have kept the suite green while every comment said the wrong thing. The suggested fix pulled each `case` branch out with a regex anchored to the exact indentation. That trades one fragility for another — any reindentation breaks the pattern and the test reports a problem that is not there — and it still reads the source rather than the output. So the tests now run the composite and read the Markdown it produced. `render_reporter(state)` executes the body with `gh` stubbed, and each state is checked for its own alert and mark *and* for the absence of the other three; exclusivity is what catches a swap. Verified by swapping IMPORTANT for NOTE on `waiting` and watching it fail. The link assertions moved the same way: they matched shell fragments like `${SERVER_URL}/${REPO}/commit/...` and now match rendered URLs, so the no-bare-mention rule holds however the composite spells it.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Addresses the third CodeRabbit review on #909. The state test read the rendered comment but checked `[!IMPORTANT]` and `## 💥 …` as separate strings. A GitHub alert only renders inside a blockquote, so dropping the `>` would leave both assertions green while the comment rendered as plain text — getting the content right and the presentation wrong, which is the thing this PR is about. Now asserted as adjacent quoted lines. Verified by removing the `>` from `waiting` and watching it fail.
Contributor
|
@coderabbitai review |
✅ Action performedReview finished.
|
6 of 14 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
2 participants
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.
Description
Follow-up to #908, from looking at the live comment on #906.
The alert block described the situation in prose, so you had to read it to know what it was. Now a 💥 sits in an
##heading inside the block, which makes the state legible from the PR timeline before anyone opens anything.The table gains marks on the three rows that matter, so it can be scanned instead of read:
And each state now gets the alert that fits it, instead of a plain heading:
[!IMPORTANT][!NOTE][!CAUTION][!WARNING]Colour and icon come from GitHub's own alert rendering; the emoji says what the block is about.
CAUTIONis reserved for the rejection, since that is the only state where something was actually taken away — the pull request was closed.Type of Change
feat: New workflow or new input/output/step in an existing workflowBreaking Changes
None. Presentation only; no input, output or job name changes.
Testing
@this-branchor the beta tagAll four states rendered locally from the composite's own shell body and read end to end.
actionlint,composite-schemaandtyposclean; 72 wiring tests, one new pinning each state to its alert and mark.Caller repo / workflow run: the comment this replaces is live on #906.
Related Issues