Automation: Require milestones on PRs #61756
Conversation
ebinnion
requested review from
ObliviousHarmony
and removed request for
a team
github-actions
bot
added
the
focus: monorepo infrastructure
Testing GuidelinesHi @ObliviousHarmony , Apart from reviewing the code changes, please make sure to review the testing instructions (Guide) and verify that relevant tests (E2E, Unit, Integration, etc.) have been added or updated as needed. Reminder: PR reviewers are required to document testing performed. This includes:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThe changes shift milestone assignment from automatic post-merge to mandatory pre-merge validation. A new GitHub Actions workflow enforces that PRs must have a milestone assigned before merging. An existing post-merge workflow and its associated PHP script are simplified by removing automatic milestone assignment logic. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant PR as Pull Request
participant GH as GitHub Actions (pr-require-milestone)
participant Script as actions/github-script
Note over PR,GH: PR events (opened/reopened/edited/synchronize/ready_for_review/milestoned/demilestoned)
PR->>GH: trigger workflow (if target == trunk and not draft)
GH->>Script: read PR metadata
Script-->>GH: milestone present?
alt milestone set
GH-->>PR: log milestone title (success)
else no milestone
GH-->>PR: fail check with message "assign a milestone"
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/pull-request-post-merge-processing.yml (1)
29-30: Update User-Agent header to reflect new purpose.The User-Agent header now contains misleading text ("set the milestone for a pull request") since milestone assignment has been moved to pre-merge validation. Update it to a more generic description of post-merge processing.
- --header 'User-Agent: GitHub action to set the milestone for a pull request' \ + --header 'User-Agent: GitHub action for post-merge pull request processing' \
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/pr-require-milestone.yml(1 hunks).github/workflows/pull-request-post-merge-processing.yml(1 hunks).github/workflows/scripts/assign-milestone-to-merged-pr.php(0 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/scripts/assign-milestone-to-merged-pr.php
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: jorgeatorres
Repo: woocommerce/woocommerce PR: 60262
File: .github/workflows/release-build-zip-file.yml:133-138
Timestamp: 2025-08-08T08:14:35.100Z
Learning: WooCommerce milestone convention: The woocommerce/woocommerce repository only creates GitHub milestones in the x.y.0 format, and those milestones cover RCs and patch releases for that minor train. In .github/workflows/release-build-zip-file.yml, the pre-build verification that sets `milestone="$version_prefix.0"` is intentional and should remain as-is; using `$version_prefix` alone would be incorrect for this repo.
Learnt from: prettyboymp
Repo: woocommerce/woocommerce PR: 59048
File: .github/workflows/cherry-pick-milestoned-prs.yml:118-131
Timestamp: 2025-07-11T13:05:08.440Z
Learning: In GitHub Actions workflows, when reviewing steps that access event data like `github.event.pull_request.milestone`, check the entire job dependency chain and conditional logic. If upstream jobs already validate the data exists (like checking `github.event.pull_request.milestone != null` in a prerequisite job), then downstream jobs don't need redundant null-checks since the data is guaranteed to exist by that point.
Learnt from: prettyboymp
Repo: woocommerce/woocommerce PR: 60272
File: .github/workflows/release-open-issue-warning.yml:81-86
Timestamp: 2025-08-08T13:59:11.524Z
Learning: Repo: woocommerce/woocommerce — In .github/workflows/release-open-issue-warning.yml, the use of `advanced_search: true` in github.rest.search.issuesAndPullRequests is intentional and temporarily required per GitHub’s 2025-03-06 Issues Advanced Search API update. Prefer keeping it (with a brief inline comment and optional fallback) until GitHub removes the requirement.
Learnt from: prettyboymp
Repo: woocommerce/woocommerce PR: 60272
File: .github/workflows/release-open-issue-warning.yml:15-23
Timestamp: 2025-08-08T13:52:26.423Z
Learning: Repository: woocommerce/woocommerce
Context: .github/workflows/release-open-issue-warning.yml and similar workflows
Learning: Maintainers (per prettyboymp) are fine requiring third-party modules inside actions/github-script without adding setup-node/createRequire. Prefer to keep this pattern and defer structural changes unless it demonstrably causes issues; minor safeguards like version pinning are acceptable if needed.
📚 Learning: 2025-07-11T13:05:08.440Z
Learnt from: prettyboymp
Repo: woocommerce/woocommerce PR: 59048
File: .github/workflows/cherry-pick-milestoned-prs.yml:118-131
Timestamp: 2025-07-11T13:05:08.440Z
Learning: In GitHub Actions workflows, when reviewing steps that access event data like `github.event.pull_request.milestone`, check the entire job dependency chain and conditional logic. If upstream jobs already validate the data exists (like checking `github.event.pull_request.milestone != null` in a prerequisite job), then downstream jobs don't need redundant null-checks since the data is guaranteed to exist by that point.
Applied to files:
.github/workflows/pr-require-milestone.yml.github/workflows/pull-request-post-merge-processing.yml
📚 Learning: 2025-08-08T08:14:35.100Z
Learnt from: jorgeatorres
Repo: woocommerce/woocommerce PR: 60262
File: .github/workflows/release-build-zip-file.yml:133-138
Timestamp: 2025-08-08T08:14:35.100Z
Learning: WooCommerce milestone convention: The woocommerce/woocommerce repository only creates GitHub milestones in the x.y.0 format, and those milestones cover RCs and patch releases for that minor train. In .github/workflows/release-build-zip-file.yml, the pre-build verification that sets `milestone="$version_prefix.0"` is intentional and should remain as-is; using `$version_prefix` alone would be incorrect for this repo.
Applied to files:
.github/workflows/pr-require-milestone.yml.github/workflows/pull-request-post-merge-processing.yml
📚 Learning: 2025-08-11T06:45:22.345Z
Learnt from: CR
Repo: woocommerce/woocommerce PR: 0
File: .cursor/rules/generate-pr-description.mdc:0-0
Timestamp: 2025-08-11T06:45:22.345Z
Learning: Applies to .github/PULL_REQUEST_TEMPLATE.md : Use .github/PULL_REQUEST_TEMPLATE.md as the basis for all PR descriptions
Applied to files:
.github/workflows/pr-require-milestone.yml
🔇 Additional comments (1)
.github/workflows/pr-require-milestone.yml (1)
1-41: Workflow structure and validation logic look solid.The new milestone-enforcement workflow correctly transitions validation from post-merge (automatic assignment) to pre-merge (required check). Trigger types are appropriate—including
demilestonedensures the requirement is maintained if a milestone is removed before merge. Draft PR exclusion prevents unintended enforcement on work-in-progress PRs. Permission level and script logic are both correct.To proceed, verify that your repository's branch protection rules on
trunkrequire the "Ensure milestone is assigned" status check to pass, so that PRs without milestones are actually blocked from merging.
ebinnion
force-pushed
the
add/github-workflow-require-milestone
branch
from
740ea3d to
8425109
Compare
|
Claude finished @ebinnion's task —— View job Backwards Compatibility ReviewTodo List:
SummaryRisk Level: None This PR contains only GitHub Actions workflow changes that affect the repository's automation infrastructure. The changes shift milestone assignment from a post-merge automatic process to a pre-merge requirement check. These modifications are purely procedural and have no backwards compatibility impact on the WooCommerce/WordPress ecosystem. Rationale:
Testing FocusNo specific BC testing required for this PR since it only affects repository automation. However, to validate the workflow changes themselves:
BC-Specific AnalysisFiles Changed:
Scope Review:
Note on Review Comments: Confidence100% - This PR contains exclusively GitHub Actions workflow files that execute on GitHub's infrastructure during CI/CD processes. These files have no runtime interaction with WordPress, WooCommerce, or the PHP/JavaScript code that runs on merchant sites. Therefore, there is zero possibility of backwards compatibility impact on the WooCommerce ecosystem. Conclusion: No breaking changes detected. This PR is safe to merge from a backwards compatibility perspective. |
|
Claude finished @ebinnion's task —— View job Backwards Compatibility ReviewTodo List:
SummaryRisk Level: None This PR contains only GitHub Actions workflow changes that affect the repository's automation infrastructure. The changes shift milestone assignment from a post-merge automatic process to a pre-merge requirement check. These modifications are purely procedural and have no backwards compatibility impact on the WooCommerce/WordPress ecosystem. Rationale:
Testing FocusNo specific BC testing required for this PR since it only affects repository automation. However, to validate the workflow changes themselves:
BC-Specific AnalysisFiles Changed:
Scope Review:
Note on Review Comments: Confidence100% - This PR contains exclusively GitHub Actions workflow files that execute on GitHub's infrastructure during CI/CD processes. These files have no runtime interaction with WordPress, WooCommerce, or the PHP/JavaScript code that runs on merchant sites. Therefore, there is zero possibility of backwards compatibility impact on the WooCommerce ecosystem. Conclusion: No breaking changes detected. This PR is safe to merge from a backwards compatibility perspective. |
There was a problem hiding this comment.
Thanks @ebinnion,
The pull request description doesn't really explain why we want to do this. We used to require milestones pre-merge and migrated to the auto-assignment that we have now because manual assignment was a chore and easy to mess up with the wrong milestone.
There was a problem hiding this comment.
In general, we should try to avoid creating new workflows if there are existing workflows that are already a good fit. This seems perfectly at home with one of our other pull request workflows, doesn't it?
Submission Review Guidelines:
Changes proposed in this Pull Request:
How to test the changes in this Pull Request:
.github/workflows/pr-require-milestone.ymlfile to trunk.Testing that has already taken place:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Changelog Entry Comment
Comment
Changelog is automation only.