-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Add workflow to check mismatching labels #4865
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughA new GitHub Actions workflow named "Check labels" has been added. This workflow is triggered on pull request events when labels are added or removed. It runs a job that checks if both "has-parent" and "current" labels are present on a pull request. If both are found, it adds a "wrong-base-branch" label, creates a review requesting changes with instructions to rebase onto the Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub
participant Workflow
participant PR Author
GitHub->>Workflow: PR label added or removed
Workflow->>GitHub: Fetch PR labels
alt "has-parent" and "current" labels present
Workflow->>GitHub: Add "wrong-base-branch" label to PR
Workflow->>GitHub: Create review requesting changes (rebase onto next)
Workflow->>GitHub: Mark workflow as failed
GitHub->>PR Author: Notify of required branch change
else Labels not both present
Workflow-->>GitHub: Do nothing
end
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.github/workflows/needs-docs.yml (3)
1-2: Workflow name is too generic
The name "Check labels" doesn’t convey the specific intent of preventing PRs with a linked code PR from targeting the wrong branch. Consider renaming to something like"Prevent merge to current when has-parent"or"Validate branch target for code PRs".
12-15: Step name and action version pinning
- The step is called “Check for needs-docs label” but it’s actually validating
has-parent+current. Rename to something likeValidate branch target for code PRs.- Pin to the major version alias (e.g.
actions/github-script@v7) instead ofv7.0.1to automatically pick critical patch updates.
16-22: Label‐lookup logic is correct but can be simplified
Using.findworks, but for pure existence checks.somereads more clearly:const hasParent = labels.some(l => l.name === 'has-parent'); const isCurrent = labels.some(l => l.name === 'current');This avoids unneeded pointer references and signals intent (boolean test).
✅ Deploy Preview for esphome ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/needs-docs.yml (1)
31-37: Review creation is well-formed
The call togithub.rest.pulls.createReviewcorrectly specifiesevent: 'REQUEST_CHANGES'and uses a clear instructional message. Consider extracting the message into a single constant to avoid drift if it changes in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/needs-docs.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
.github/workflows/needs-docs.yml (1)
Learnt from: jesserockz
PR: esphome/esphome-docs#4865
File: .github/workflows/needs-docs.yml:0-0
Timestamp: 2025-05-01T03:29:47.895Z
Learning: In the esphome-docs repository, the "current" label is automatically added by a bot to pull requests, making it a reliable indicator for the target branch.
🔇 Additional comments (4)
.github/workflows/needs-docs.yml (4)
7-11: Job configuration looks solid
Theruns-on: ubuntu-latestandjobs.checkstructure is clear and follows best practices for single‐step actions.
12-15: Using actions/github-script@v7.0.1 is appropriate
This action conveniently exposesgithub,context, andcorewithin the script block, simplifying REST calls.
16-23: Label retrieval logic is correct
The code properly lists labels on the issue/PR and checks for both'has-parent'and'current'. Relying on the bot‐added"current"label is safe per the repository convention.
24-29: Verify existence of thewrong-base-branchlabel
Adding a label that doesn’t yet exist will cause the step to fail. Confirm that thewrong-base-branchlabel is pre-created in this repository or add a prior step to create it if missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this is a feature matched with a PR in https://github.com/esphome/esphome, please target your PR to the next branch and rebase.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Description:
Many times I have accidentally merged a PR but it was targetted to current instead of next.
This workflow will fail if the description has a code pr linked and targets current.
Related issue (if applicable): fixes
Pull request in esphome with YAML changes (if applicable):
Checklist:
I am merging into
nextbecause this is new documentation that has a matching pull-request in esphome as linked above.or
I am merging into
currentbecause this is a fix, change and/or adjustment in the current documentation and is not for a new component or feature.Link added in
/components/index.rstwhen creating new documents for new components or cookbook.