Conversation
grunt-cli and gulp-rename were used by the Gruntfile/gulpfile but never declared as dependencies, so `npm run makepot`/`compress` and `npm run styles` failed outright on a clean install regardless of the grunt/gulp versions installed. Also bumps grunt and grunt-contrib-* packages, fixes compress excluding composer files from the release zip, fixes makepot's PHP 8 create_function() crash, and keeps Uglify output ES5 with license headers preserved. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
phpcs.xml's text_domain was set to "cenote" -- a different ThemeGrill theme, clearly copy-pasted in -- and referenced sniffs/properties (WordPress.VIP.RestrictedFunctions, DeprecatedFunctions' minimum_supported_version) removed in modern WPCS, which made phpcs error out entirely rather than just report stale findings. Fixed the text domain, dropped the obsolete rules, excluded the bundled third-party Kirki library from linting, and bumped composer.json to current squizlabs/wpcs/phpcompatibility-wp versions with phpcs/phpcbf scripts. Adds a PR workflow that lints changed PHP files only (added/copied/modified/renamed, not deleted). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The new PHPCS workflow uses an incorrect PHP glob (**.php) and the makepot header/template has a malformed language-team value, both of which should be corrected before merging.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (2)
What changed in this PR
This PR repairs and modernizes the Flash theme’s build/tooling pipeline (npm/grunt/gulp) and updates the PHP coding standards setup (Composer + WPCS/PHPCS), including adding a CI PHPCS workflow and regenerating the theme POT file.
Changes:
- Updated Node/Grunt/Gulp/Stylelint dependency versions and added missing JS build dependencies to restore a working
npm run build. - Modernized PHPCS/WPCS Composer tooling and updated
phpcs.xmlconfiguration (including PHPCompatibilityWP and exclusions). - Added a GitHub Actions workflow to run PHPCS on changed PHP files and regenerated
languages/flash.pot.
| File | Description |
|---|---|
phpcs.xml |
Updates ruleset, text domain, PHPCompatibilityWP config, and exclusions. |
package.json |
Fixes missing deps and modernizes build tool versions; raises Node/npm engines. |
Gruntfile.js |
Updates uglify options, POT header template, and zip exclusions. |
languages/flash.pot |
Regenerated POT output from the updated toolchain. |
composer.json |
Updates PHPCS/WPCS-related dev dependencies and adds phpcs/phpcbf scripts. |
composer.lock |
Locks updated PHPCS/WPCS toolchain dependency graph. |
.npmrc |
Adds npm install behavior override (omit=optional). |
.github/workflows/phpcs.yml |
Adds PR-time PHPCS checks scoped to changed PHP files. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| pull_request: | ||
| paths: | ||
| - '**.php' |
There was a problem hiding this comment.
Checked this against both tools' own docs rather than changing it on faith: GitHub's own path-filter syntax docs give **.js as the documented example for "all .js files in the repository" (i.e. it already matches nested paths, no leading **/ needed) — same semantics apply to **.php in the on.pull_request.paths filter. And tj-actions/changed-files' own README (the files: input used in the second step) has an identical example, **.md, specifically captioned "Get all changed markdown files" including nested ones under docs/ — it explicitly adopted GitHub's native glob semantics as of v13, not minimatch's. So **.php already matches nested PHP files under both tools as written. Switching to **/*.php would be redundant at best, and in some glob dialects **/*.php actually fails to match root-level .php files (no directory segment before the file) — which would be a real regression for a WordPress theme, where most template files (front-page.php, single.php, etc.) live at the theme root. Leaving as-is unless you have a specific counter-example of a nested file this pattern misses.
| potComments: 'Copyright (C) {year} ThemeGrill\nThis file is distributed under the GNU General Public License, version 3 (GPLv3).', | ||
| potHeaders: { | ||
| 'report-msgid-bugs-to': 'themegrill@gmail.com', | ||
| 'language-team': 'ThemeGrill <themegrill@gmail.com' |
There was a problem hiding this comment.
Fixed in 50a7868 — makepot's exclude list only had deploy/.; added inc/kirki/. alongside it. Regenerated the .pot: Kirki-related string count went from 217 matches to 0, confirmed with grep -ci kirki languages/flash.pot before/after.
Supersedes the stalled PR #105, which used yarn (this repo has never used yarn -- no yarn.lock exists, npm/package-lock.json is what's actually used) and predates this branch's PHPCS/composer.json setup. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
#build-zip
package-lock.json isn't committed here (matches accelerate-pro's own .gitignore convention), so npm ci has no lockfile to install from and fails with EUSAGE. #build-zip Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Build for ⬇️ Download flash.zip (3.1M) Installs directly via Plugins → Add New → Upload Plugin. A later push only rebuilds this if its commit message includes |
makepot only excluded deploy/, so third-party Kirki UI/library strings (font weights, control labels, etc.) were leaking into languages/flash.pot alongside this theme's own translatable strings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
style.css declares Requires PHP: 5.6, but testVersion was set to 7.4- -- silently raising the theme's real compatibility contract without that being a deliberate product decision. Set to 5.6- to match what's actually declared; bumping the real minimum is a separate call for the team to make, not something to smuggle into tooling setup. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>


Summary
Fixes a completely broken build (missing dependencies, PHP 8 crash) and modernizes/repairs the existing (broken) WPCS setup for the Flash theme.
Changes
package.jsongrunt-cli@^1.4.3todependenciesnpm run makepot/compressfailed withgrunt: command not foundon a clean install regardless of any other fixpackage.jsongulp-rename@^2.1.0todevDependenciesgulpfile.jsrequires it, but it was never declared —npm run stylesfailed withCannot find module 'gulp-rename'package.jsonstylelint-config-wordpress^12.0.0→^13.0.0stylelint@^9.2.1(already declared) satisfies neitherstylelint-config-wordpress@12's peer (^7.10.1) norgulp-stylelint@^7.0.0's peer (^9.0.0) at the same time.^13.0.0requiresstylelint@^9.1.3, compatible with both, fixing theERESOLVEfailure with the smallest possible changepackage.jsongrunt0.4.5→^1.6.3,grunt-contrib-compress^1.2.0→^2.0.0,grunt-contrib-jshint~0.11.3→^3.2.0,grunt-contrib-uglify^0.10.1→^5.2.2,grunt-contrib-watch~0.6.1→^1.1.0npm run build(styles/uglify go through gulp) — they're still unconditionally loaded atGruntfile.jsinit and would breakmakepot/compresstoo if they failed to loadpackage.jsongrunt-wp-i18n~0.5.3→^1.0.4makepot.phpcalls the removed PHP 8 functioncreate_function(), fatal-erroringgrunt makepotpackage.jsonengines.node/engines.npm>=8.9.3/>=5.5.1→>=20.0.0/>=9.0.0grunt-wp-i18n@1.0.4's lockednode-wp-i18n@1.2.8→glob@11.1.0requires Node 20 or ≥22Gruntfile.jscompresstask now also excludescomposer.json,composer.lockcomposer.json/kirki-composer/autoloader underinc/kirki/is untouched — that's genuinely needed at runtime)Gruntfile.jsmakepottask now setspotCommentsexplicitlygrunt-wp-i18n@1.0.4's default comment template leaks a literal<!=...=!>placeholder into the.potheader when not overridden — an upstream bugGruntfile.jsuglifytask usescompress.arrows: falseandoutput.commentsinstead ofpreserveCommentspreserveCommentswas removed ingrunt-contrib-uglify2.x+, and the new major emits ES2015 arrows by default. (This grunt task isn't wired intonpm run buildcurrently —gulp-uglifyhandles the shippedjs/*.min.js, and produces clean ES5 with no changes needed — but fixed for anyone runninggrunt default/grunt devdirectly).npmrc(new)omit=optionalnpm installsilently skips alldevDependencieswhenever the shell hasNODE_ENV=productionsetcomposer.jsonsquizlabs/php_codesniffer3.2→^3.9,wp-coding-standards/wpcs0.14.0→^3.1,wimg/php-compatibility→phpcompatibility/phpcompatibility-wp^2.1,dealerdirect/phpcodesniffer-composer-installer^0.7.2→^1.0; addedphpcs/phpcbfscriptsphpcs.xmltext_domainwas"cenote"— fixed to"flash"style.css) isflashphpcs.xmlWordPress.VIP.RestrictedFunctionsexclude and the wholeWordPress.WP.DeprecatedFunctionsoverrideERROR: Referenced sniff ... does not exist), not just report stale findingsphpcs.xmltestVersion5.2-99.0→7.4-,PHPCompatibility→PHPCompatibilityWPphpcs.xmlexclude-patternforinc/kirki/*.github/workflows/phpcs.yml(new)languages/flash.potnpm run buildon the fixed toolchainQA / Test steps
Toolchain (no WordPress needed):
git clonethis branch fresh,cdinto it.npm installwith no flags.ERESOLVEerror, and bothnode_modules/.bin/gruntandnode_modules/.bin/gulpexist.npm run build.styles(gulp/Sass),uglify(gulp),makepot(grunt), andcompress(grunt) all complete, exit code0. NoCannot find module 'gulp-rename', nogrunt: command not found, nocreate_function()PHP fatal error.languages/flash.potand check the first 2 lines.# Copyright (C) <year> ThemeGrill/# This file is distributed under the GNU General Public License, version 3 (GPLv3).— plain text, no<!=or=!>characters.grep -c '=>' js/*.min.js.0for every file — no ES2015 arrow functions in the minified output.unzip -l dist/flash.zip | grep -E 'flash/(composer\.(json|lock)|phpcs\.xml)$'.inc/kirki/composer.jsonetc. are expected to remain — that's runtime code, not our tooling).composer install && composer phpcs.phpcs.xml(previously errored out entirely with "Referenced sniff ... does not exist"). Reports the pre-existing ~593-error backlog — expected and out of scope for this PR.Theme behavior in WordPress (manual — do this before merging):
dist/flash.zipfrom step 3 above (or activate the theme directory directly) and activate Flash on a WordPress install.Loco Translate(or similar) can still readlanguages/flash.potwithout errors.Changelog entry
Fix - Fixed a broken npm build (missing dependencies) and modernized/repaired the theme's coding-standards tooling.
🤖 Generated with Claude Code