Run make-pot on manual dispatch only, and drop the WordPress+MySQL scaffolding - #1846
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe workflow is manually dispatched, generates the POT directly from the built distribution using PHP and WP-CLI, compares it with a baseline, and uses the distribution output for artifact upload and staging. Pull-request branch and base selection now use the workflow ref. ChangesPOT generation workflow
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/make-pot.yml:
- Around line 27-29: Update the concurrency group expression in the workflow’s
concurrency block to mirror the resolve step’s BASE precedence: inputs.base,
then inputs.ref, then github.head_ref, then github.ref_name. This ensures runs
resolving to the same BASE share one concurrency group.
- Around line 100-112: Update the “Verify the PR base is a real branch” step to
preserve and surface the gh api failure details instead of suppressing stderr.
Keep the invalid-branch guidance for a confirmed missing branch, but distinguish
authentication, rate-limit, network, and other API failures so they are reported
with the original diagnostic context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cab7ec8a-ebac-46bf-be43-995dca496631
📒 Files selected for processing (1)
.github/workflows/make-pot.yml
ec1bb80 to
9c1fbe3
Compare
pattonwebz
changed the title
`wp i18n make-pot` is static analysis over the source tree: it parses PHP/JS for translation calls and writes a .pot. It never bootstraps WordPress and never touches a database. Verified by running it against this plugin on a machine with no WordPress, no MySQL and no wp-config.php - exit 0. So the mysql:5.7 service, the readiness loop, the wordpress.org tarball download, the wp-config sed, `wp core install`, and the copy into wp-content/plugins and back out were all satisfying a dependency that does not exist. wp-cli now comes from setup-php's `tools:`, replacing the apt PHP install and the phar download. The xdebug.max_nesting_level line went with it: that apt line never installed xdebug, and `coverage: none` keeps it off. make-pot still runs against dist/accessibility-checker and still writes into that copy's languages/ folder, exactly as before, so the generated POT is unchanged. Because the committed copy at that path is also the baseline the diff step compares against, it is stashed to $RUNNER_TEMP first - otherwise the new POT would be compared against itself and never report a change. No behaviour change otherwise: same triggers, same `base` input, same PR targeting, same single .pot in add-paths. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019LoLgW7oFjBPxGea9kiZJ2
9c1fbe3 to
87dae18
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/make-pot.yml (1)
55-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider pinning the
wp-clitool version.
tools: wp-cliinstalls whatever the latest wp-cli release is at run time. Since theX-Generatorline is excluded from the diff anyway, this won't cause false diffs today, but an unpinned wp-cli means a future release could changemake-pot's output format (headers, quoting, comment style) and silently break the exclusion regex in the next step, or introduce unrelated CI breakage with no easy rollback.♻️ Suggested pin
- name: Set up PHP with wp-cli uses: shivammathur/setup-php@v2 with: php-version: '8.2' - tools: wp-cli + tools: wp-cli:2.12.0 coverage: none🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/make-pot.yml around lines 55 - 60, Pin the wp-cli tool version in the setup-php step’s tools configuration instead of installing the latest release dynamically. Use the project’s intended or currently validated wp-cli version so make-pot output and CI behavior remain reproducible.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/make-pot.yml:
- Around line 55-60: Pin the wp-cli tool version in the setup-php step’s tools
configuration instead of installing the latest release dynamically. Use the
project’s intended or currently validated wp-cli version so make-pot output and
CI behavior remain reproducible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d883a7e3-1d21-49dc-9d30-9b2caa30e205
📒 Files selected for processing (1)
.github/workflows/make-pot.yml
|
Note for reviewers: CodeRabbit's two inline comments above are stale — they're anchored to commit Both target code that no longer exists here. The original version of this PR also made the workflow accept arbitrary
Line 29 is now The PR as it stands is a pure removal: |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Drops the pull_request trigger, so this workflow no longer fires off any branch or PR - the only way to run it is Actions > Generate POT file > Run workflow. Three things fall out of that automatically, since none of them can fire any more: - The job's `if:` gate. It existed to filter pull_request events down to automatic_translations_* and tm_edits_* head branches; with no automatic event left there is nothing to filter, so there is no branch identification in this workflow at all now. - The "Comment on PR if POT file changed" step, which was conditioned on github.event.pull_request. - Every github.head_ref reference, which is only ever set on a pull_request. PR targeting is unchanged for a dispatch run: `branch` was already `head_ref || ref_name` and head_ref is empty on dispatch, so it is now just ref_name; `base` keeps the same `inputs.base || ref_name` precedence with its develop default. 108 lines, 12 steps. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019LoLgW7oFjBPxGea9kiZJ2
pattonwebz
changed the title
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/make-pot.yml (1)
45-50: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSet up PHP/Composer before building the distribution.
npm run dist:dotorgruns before this step, while the suppliedpackage.jsoncontract shows that it invokes Composer. The build therefore relies on whatever PHP/Composer versions happen to be preinstalled onubuntu-latest, rather than the declared PHP 8.2 toolchain. Move this setup step beforeBuild plugin(or add an explicit PHP/Composer setup before it). The runner image contents are updated regularly, andsetup-phpsupports installing Composer alongside WP-CLI. (github.com)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/make-pot.yml around lines 45 - 50, Move the “Set up PHP with wp-cli” step, including its PHP 8.2 and tool configuration, before the “Build plugin” step that runs npm run dist:dotorg. Ensure Composer is explicitly installed alongside WP-CLI so the build uses the declared PHP/Composer toolchain rather than runner defaults.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/make-pot.yml:
- Around line 45-50: Move the “Set up PHP with wp-cli” step, including its PHP
8.2 and tool configuration, before the “Build plugin” step that runs npm run
dist:dotorg. Ensure Composer is explicitly installed alongside WP-CLI so the
build uses the declared PHP/Composer toolchain rather than runner defaults.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6859fbd9-61c0-4806-bf67-f707ed424c66
📒 Files selected for processing (1)
.github/workflows/make-pot.yml
|
@coderabbitai full review |
✅ Action performedFull review finished. |
CodeRabbit caught this on the multisite PR and it applies identically here: `base` only set the PR target, while actions/checkout took no ref at all. Since create-pull-request commits onto whatever HEAD it finds, dispatching from feature/foo with base develop would build feature/foo and then raise a PR into develop containing every commit between the two - not just the POT change. Checkout now uses the same `base` expression, so one value decides what gets built, what the PR branch is named, and what it targets. There is no combination of inputs that makes them disagree. The develop default is dropped from the input at the same time: with base now driving the checkout, a hardcoded default would silently ignore the branch picked in the dispatch UI. Empty falls back to the dispatched branch, so the picker behaves as expected and build and target still match. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019LoLgW7oFjBPxGea9kiZJ2
Flagged by CodeRabbit on the multisite PR; applies identically here. The job runs `npm install` and the webpack/composer dist build against the checkout, and actions/checkout persists the GITHUB_TOKEN into .git/config by default, so that build executes with a usable push credential sitting in the working tree. Verified this is safe rather than assuming it: create-pull-request calls GitConfigHelper.configureToken() with its own `token` input (defaulting to github.token) and sets its own http.<host>/.extraheader before pushing - it even saves and unsets any persisted extraheader first, then restores it afterwards. So the PR step supplies its own auth and does not read what checkout persisted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019LoLgW7oFjBPxGea9kiZJ2
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 38 minutes. |
CodeRabbit caught this on the export PR and it applied to all six repos. The filter excluded every line starting with "#", which is not just the volatile "#:" source references - it also threw away "#." extracted translator comments and "#," flags such as php-format. A change confined to those lines reported pot_changed=false, so no PR was raised and the committed POT stayed stale in a way that matters to translators. Filtering "#:" alone gets all three cases right. Verified against a real POT: source line numbers shifted -> false (still correctly ignored) translators comment changed -> true (was false) php-format flag added -> true (was false) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019LoLgW7oFjBPxGea9kiZJ2
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 49 minutes. |
1 participant
Why
wp i18n make-potis pure static analysis over the source tree: it parses PHP/JS for translation calls and writes a.pot. It never bootstraps WordPress and never touches a database.Verified rather than assumed — running it against this plugin on a machine with no WordPress, no MySQL and no
wp-config.phpexits 0. So this workflow'smysql:5.7service, readiness loop, wordpress.org tarball download,wp-configsed,wp core install, and the copy intowp-content/pluginsand back out were all satisfying a dependency that does not exist.What changed
Net −54 / +16 lines, six steps removed:
mysql:5.7service, "Wait for MySQL to be ready", "Download and configure WordPress", "Install WordPress", the apt PHP install, and thewp-cli.phardownload.wp-clinow comes fromshivammathur/setup-php'stools:, matching howbuild-plugin-with-ref.ymlalready sets up PHP in this repo.xdebug.max_nesting_levelline went with it: that apt line never installed xdebug, andcoverage: nonekeeps it off.--allow-rootdropped — the runner user isn't root.What deliberately did not change
if:gating onautomatic_translations_*/tm_edits_*.baseinput and same PR targeting expressions..potinadd-paths. Nothing generates.json—make-potwrites one file, and JSON comes from the external translation system.make-potstill runs againstdist/accessibility-checkerand still writes into that copy'slanguages/folder, so the generated POT is unchanged.One detail worth reviewing
The committed POT at
dist/accessibility-checker/languages/accessibility-checker.potis both the filemake-potoverwrites and the baseline the diff step compares against. So it's stashed to$RUNNER_TEMPbefore generation:Without that, the new POT would be compared against itself and
pot_changedcould never reporttrue.The source argument stays
.(the plugin root) rather than thelanguages/folder, becausemake-potresolves its#:source references against the source argument, not the working directory — verified by generating the same plugin from both locations and diffing: byte-identical.Testing
actionlintclean; YAML parses.wordpress/,latest.tar.gz,wp-config,wp core install,--allow-root,wp-cli.phar, xdebug orapt-get.accessibility-new-window-warningsplugin (small enough to run locally): the baseline is stashed byte-intact, the POT regenerates in place,pot_changed=falseis correctly reported, and no.po/.mo/.jsonfile is touched.make-potjob reportsskippingon this PR, since the branch isn't a translations branch. Worth dispatching once after merge to confirm it reportspot_changed=false.Sibling PRs for
accessibility-checker-proandaccessibility-new-window-warningsto follow.accessibility-checker-multisitehas the same scaffolding plus awp i18n make-jsonstep, so it's left for a separate change.🤖 Generated with Claude Code
https://claude.ai/code/session_019LoLgW7oFjBPxGea9kiZJ2
Summary by CodeRabbit