Add a real dist build, and generate the POT from it - #56
Open
pattonwebz wants to merge 3 commits into
Open
Conversation
Replaces the manual packaging recipe in .claude/skills/package-plugins with a build script, matching accessibility-checker and its add-ons, then bolts the POT workflow onto it. ## The dist build `npm run dist` -> dist/boardscribe-<version>.zip, via scripts/dist.sh, the same shape as the other plugins: wp-scripts plugin-zip against package.json's new `files` allowlist, unpacked to dist/boardscribe/, then rezipped with the version in the name and a single top-level directory named for the slug. `dist:keep-build-folder` leaves dist/boardscribe/ behind for CI to scan. The allowlist encodes the manifest the skill had verified by hand, and a real run reproduces that manifest exactly - same 26 shipped files, with vendor/ reduced to autoload.php + composer/. Two things the allowlist cannot express, so the script handles them: - npm force-packs package.json and README.md whatever `files` says. Caught by running the build and reading `unzip -l`, not by reasoning about it - the first zip shipped both. (LICENSE is force-packed the same way, and that one belongs in the zip.) - .DS_Store stripping, previously a manual step in the skill. The script also refuses to package when vendor/ holds anything beyond autoload.php and composer/, which is the failure the skill warned about in prose: a zip built from a dev vendor/ is both bloated and wrong. Verified by running dist.sh with dev dependencies installed - it exits 1 and names them. ## The POT workflow .github/workflows/make-pot.yml is the same file the six sibling repos now run, differing only by slug: manual dispatch only, no WordPress or MySQL (make-pot is static analysis and needs neither), wp-cli from setup-php, the generated POT diffed against the committed one while ignoring only `#:` reference locations, and a PR raised against `base` - which also drives the checkout, so the tree it builds and the branch it targets can never disagree. It scans dist/boardscribe/, which changes where the POT's references point: src/js/* today, assets/build/* once this runs, because src/ does not ship and the bundles do. The strings themselves are unaffected - wp-scripts' default Terser config already preserves `translators:` comments, confirmed present in the POT generated from the built bundles. Expect the first dispatch to raise a translations PR: the committed POT dates from 2026-07-16 and yields 88 msgids, while the current tree yields 122. ## The skill Its free-plugin recipe is now "run npm run dist". The invariants it documented are kept - why vendor/ ships, the deliberate choice to leave composer.json out and accept the Plugin Check warning (PRO-1196), the single-top-level-directory rule - since those are decisions, not procedure. Pro keeps its manual recipe until it gets a dist.sh of its own. Also corrects the manifest's assets/images/logo.png to logo.svg, which the real build shows is what actually ships. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019LoLgW7oFjBPxGea9kiZJ2
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 38 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: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThe PR adds a scripted plugin distribution pipeline, restricts packaged contents, updates free and Pro release guidance, and introduces a manually triggered GitHub Actions workflow for generating and proposing POT file updates. ChangesRelease automation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Maintainer
participant Actions as GitHub Actions
participant Build as npm run dist:keep-build-folder
participant WPCLI as wp i18n make-pot
participant PR as create-pull-request
Maintainer->>Actions: Dispatch workflow with optional base
Actions->>Build: Build plugin and retain staging folder
Build-->>Actions: Provide generated plugin tree
Actions->>WPCLI: Generate boardscribe.pot
WPCLI-->>Actions: Return generated POT
Actions->>PR: Create or update POT pull request when changed
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Pro gets the same scripts/dist.sh in equalizedigital/boardscribe-pro#35, so the skill no longer carries a manual bash recipe for it - both plugins are now `npm run dist`. Its Pro manifest was also out of date, which the real build exposed: includes/ has gained DocumentPicker, DocumentCPT, MeetingDocumentRelationship and MeetingType, assets/css has gained admin-document-picker.css and lost calendar-templates.css, and src/js/front-end/calendarTemplates.js is gone. Rules 6 and 7 described the hand-run bash block (the mktemp/trap footgun) and go with it. In their place, a note that both plugins keep a committed languages/<slug>.pot, since make-pot.yml diffs against it and raises a PR on every run without one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019LoLgW7oFjBPxGea9kiZJ2
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.distignore:
- Around line 2-5: Add composer.json to the exclusion entries in .distignore so
the denylist matches the release manifest’s forbidden files and the documented
packaging contract.
In @.github/workflows/make-pot.yml:
- Around line 14-16: Update the make-pot workflow’s make-pot job permissions to
explicitly grant contents: write and pull-requests: write, enabling
peter-evans/create-pull-request to push changes and create or update the pull
request with GITHUB_TOKEN.
In `@package.json`:
- Line 23: Update the package.json dist script so the final composer install
restoration step runs regardless of scripts/dist.sh success, while preserving
and returning the packaging command’s failure status. Keep the initial composer
install --no-dev --optimize-autoloader prerequisite and existing build/package
sequence intact.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 76235f6b-382a-4197-a35c-f2a3ccdf5660
📒 Files selected for processing (5)
.claude/skills/package-plugins/SKILL.md.distignore.github/workflows/make-pot.ymlpackage.jsonscripts/dist.sh
…failure Addresses the review on this PR, plus a mistake of my own that it surfaced. I read the packaging skill while the repo was still checked out on an older feature branch, then rewrote it after switching to main - silently reverting two decisions that had landed in between: - d8113f7 ships composer.json (reversing PRO-1196) so Plugin Check's missing_composer_json_file finding is clear for the WP.org submission. My rewrite put the old "keep it out" text back, and my `files` allowlist left composer.json out of the zip, which would have reintroduced the warning. - 13214ed's rule 5 records that guideline 4 is answered by keeping equalizedigital/boardscribe public and linking it from readme.txt, and that src/, webpack.config.js and package.json must start shipping if it ever goes private again. My rewrite deleted that paragraph. The skill is now rebuilt from main's copy with the dist-build changes applied on top, so both decisions survive verbatim. composer.json is in the allowlist and confirmed in the zip; composer.lock stays out, as that note requires. The vendor guard used GNU `find -printf` with stderr sent to /dev/null. On macOS or BSD that option does not exist, so the command fails, the output is empty and the guard passes silently - the exact failure it exists to catch, on the platform this script already strips .DS_Store for. Replaced with a portable glob-and-case loop, verified to still fail on a dev vendor/ and to name the offending packages. `npm run dist` chained the restoring `composer install` with &&, so a packaging failure skipped it and left the working tree without phpcs/phpunit. It now runs unconditionally and the packaging exit status is preserved. Also declares contents/pull-requests write on the workflow. create-pull-request pushes with GITHUB_TOKEN, this repo has never had a bot-authored PR to prove the default is write, and I cannot read the repository default with my token - wp-version-checker.yml already declares its permissions, so this matches local convention rather than relying on an unverifiable default. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019LoLgW7oFjBPxGea9kiZJ2
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
1 participant
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.
Replaces the manual packaging recipe in
.claude/skills/package-pluginswith a build script matching accessibility-checker and its add-ons, then bolts the POT workflow onto it.The dist build
scripts/dist.shis the same shape as the sibling plugins:wp-scripts plugin-zipagainst a newfilesallowlist inpackage.json, unpacked todist/boardscribe/, then rezipped with the version in the name and a single top-level directory named for the slug.The allowlist encodes the manifest the skill had verified by hand, and a real run reproduces it exactly — same 26 shipped files,
vendor/reduced toautoload.php+composer/.Two things the allowlist can't express
package.jsonandREADME.mdwhateverfilessays. The first zip I built shipped both. Found by running the build and readingunzip -l, not by reasoning about it — this is why the siblingdist.shscripts carry twormlines that look redundant until you hit this. (LICENSEis force-packed the same way, and that one belongs in the zip.).DS_Storestripping, previously a manual step in the skill.A guard, not just a script
dist.shrefuses to package whenvendor/holds anything beyondautoload.phpandcomposer/— the failure the skill warned about in prose. Verified by running it with dev dependencies installed:The POT workflow
.github/workflows/make-pot.ymlis the same file the six sibling repos now run, differing only by slug: manual dispatch only, no WordPress or MySQL (make-pot is static analysis and needs neither), wp-cli fromsetup-php, the generated POT diffed against the committed one while ignoring only#:reference locations, and a PR raised againstbase— which also drives the checkout, so the tree it builds and the branch it targets can't disagree.Two consequences worth knowing before merging
Where POT references point changes. It scans
dist/boardscribe/, so references move fromsrc/js/*toassets/build/*—src/doesn't ship, the bundles do. The strings themselves are unaffected; wp-scripts' default Terser config already preservestranslators:comments (comments: /translators:/i,extractComments: falsein its webpack config), and I confirmed they're present in a POT generated from the built bundles.The first dispatch will raise a translations PR. The committed POT dates from 2026-07-16 and yields 88 msgids; the current tree yields 122. That's the workflow working, not a fault.
The skill
Its free-plugin recipe is now "run
npm run dist". The invariants it documented are kept — whyvendor/ships, the deliberate choice to leavecomposer.jsonout and accept the Plugin Check warning (PRO-1196), the single-top-level-directory rule — because those are decisions, not procedure. Pro keeps its manual recipe until it gets adist.shof its own; deleting it would have left Pro with no documented packaging at all.Also corrects the manifest's
assets/images/logo.pngtologo.svg, which the real build shows is what actually ships, and repoints.distignore's header comment at the allowlist as the canonical source.Testing
npm run distrun end to end; manifest compared against the skill's known-good list;vendor/confirmed to be autoloader-only; dev tooling confirmed restored afterwards.bash -n scripts/dist.sh,actionlintand YAML parse all clean.mainwithbaseempty after merge, expecting the translations PR described above.🤖 Generated with Claude Code
https://claude.ai/code/session_019LoLgW7oFjBPxGea9kiZJ2
Summary by CodeRabbit