Repository navigation
fix(pr-validation): make the breaking-change comment read as information - #907
Merged
Merged
Conversation
The waiting comment opened with a siren and spent four paragraphs explaining itself. A breaking change is often deliberate, and a comment that shouts teaches people to scroll past it — which is the one thing a gate cannot afford. Now: what was found, a link straight to the decision, and who can make it. Everything else — the scope rule, the acknowledgement state, how to undo the break — is one small line at the bottom. Eighteen lines down to nine. Four things are links now where there were none: the offending commit, the pending deployment, the reviewers, and the docs. "Who can approve this" was the obvious next question and the answer was buried in Environment settings nobody thinks to open. Reviewers render as links, never as @mentions: the pending deployment already notifies exactly those people.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe workflow passes configured reviewers, or reviewers derived from base-branch CODEOWNERS, to the approval reporter. The reporter accepts reviewer and URL inputs, generates linked references, and updates its comments for waiting, approval, rejection, and unresolved decisions. ChangesBreaking-change approval reporting
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~12 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. (2 skipped: 2 … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly identifies the primary change: revising the breaking-change approval comment. It is concise and related to the changeset. Description check The description includes the required sections and explains the behavior changes, affected components, breaking-change status, testing performed, and the absence of a real caller workflow run. The Rel… 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.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. (2 skipped: 2 unsupported.)
- 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
Comment @coderabbitai help to get the list of available commands.
lerian-studio
added
size/S
Contributor
🔍 PR Validation Summary✅ PR Mergeable — no blocking failures
|
Contributor
🔍 Lint Analysis
|
Contributor
🛡️ CodeQL Analysis ResultsLanguages analyzed: Found 1 issue(s): 1 Medium
🔍 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 #907. environment-protection now reads back who the Environment is actually gated by and exposes it, and the reporter prefers that over the configured list. The two differ whenever the Environment already existed or was edited since — and naming the wrong approvers is worse than naming none, which is what the comment does when neither is available. Also from the review: the `reviewers` description still said "rendered as GitHub mentions", written before the switch to links and never updated the README gave `docs-url` as "the gate's docs page" where .coderabbit.yml asks for input tables to match action.yml exactly the example's links all pointed at `(#)` the no-bare-mention test asserted one forbidden spelling, so it passed by accident and would keep passing if the mention were written any other way. It now checks that every `@${owner}/` sits inside Markdown link text — verified by injecting a bare mention and watching it fail
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.
… unknown Addresses the second CodeRabbit review on #907. The composite read the Environment twice: once for the protection rules, once for the reviewers, with the second call swallowing failure. That allowed a state of "protected, reviewers unknown" — and an empty reviewer list is indistinguishable from "this Environment has none", so the reporter fell back to the configured list and could name people who cannot actually approve. Which is the very thing the previous commit set out to fix. The suggestion was to carry the read failure as a distinct status. Removing the state is better than reporting it: one GET now answers both questions, so either both are known or the step fails. It also drops a call to an API this job already talks to twice. Test pins it to exactly one read.
lerian-studio
added
size/M
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.
GitHub Actions Shared Workflows
Description
Rewrites the pull request comment posted by the breaking-change approval gate (#904). Observed on #906, the waiting comment opened with a siren and spent four paragraphs explaining itself, with no link anywhere.
A breaking change is often deliberate. A comment that shouts about it teaches people to scroll past it — which is the one thing a gate cannot afford.
Before — 18 lines, 0 links. After — 9 lines, 4 links:
What changed:
<sub>lines: there for whoever needs them, out of the way of whoever does not.The approved and rejected states shrank the same way (3 and 5 lines).
Reviewers render as links, never as
@mentions. The pending deployment already notifies exactly those people; a mention would ping the whole team a second time for the same thing.Affected:
src/notify/breaking-change-approval-reporter(newreviewers,server-url,docs-urlinputs),breaking-change-approval.yml(passes the reviewers through).Type of Change
feat: New workflow or new input/output/step in an existing workflowfix: Bug fix in a workflow (incorrect behavior, broken step, wrong condition)perf: Performance improvement (e.g. caching, parallelism, reduced steps)refactor: Internal restructuring with no behavior changedocs: Documentation only (README, docs/, inline comments)ci: Changes to self-CI (workflows under.github/workflows/that run on this repo)chore: Dependency bumps, config updates, maintenancetest: Adding or updating testsBREAKING CHANGE: Callers must update their configuration after this PRBreaking Changes
None. The three new composite inputs are optional with defaults, and no workflow input, output or job name changes.
Testing
@this-branchor the beta tagAll three comment states were rendered locally from the composite's own shell body and read end to end — that output is what the Description quotes.
actionlint,composite-schema,yamllintandtyposclean. 66 wiring tests pass, including a new one that pins the four links and refuses a bare@mentioncreeping back.Caller repo / workflow run: the comment this replaces is visible on #906.
Related Issues