Repository navigation
refactor: align Makefile target names with a documented grammar - #705
Merged
Merged
Conversation
github-actions
Bot
added
the
[Type] Task
XCFramework BuildThis PR's XCFramework is available for testing. Add the following to your .package(url: "https://github.com/wordpress-mobile/GutenbergKit", branch: "pr-build/705")Built from f0866db |
dcalhoun
added this pull request to stack #715
September 21, 2026 12:24
The `.PHONY` line named `lint-fix-js`, a target that does not exist, so the real `lint-js-fix` target was never declared phony and a stray file of that name would silently skip the recipe. Introduced in #237. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Target names had drifted into three competing shapes (verb-first, noun-first, and platform-last), and the elision rule was never stated, which is how `test-e2e` came to mean "web only" while reading as a superset of the platform-specific E2E targets. Recording the grammar in the Makefile header rather than AGENTS.md keeps it at the point of use: it governs exactly one file, and it lands in the diff whenever someone adds a target. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four descriptions were misleading: `build` implied native compilation when it builds web assets and copies them, `test-android` claimed all Android tests when it runs only the library module's JVM unit tests, and `test-e2e`/`test-js` both read as "JavaScript tests" with nothing distinguishing web E2E from unit runs. Also names the tool behind each lint/test target, which `lint-android` already did but its siblings did not, and settles on "E2E" over "instrumented" for the same Android concept. `preview` gains a `build` prerequisite: `vite preview` only serves `dist/`, so it served nothing on a clean checkout. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Five targets did not fit `<verb>-<scope>[-<subscope>][-<kind>][-<mode>]`: `npm-dependencies` and `e2e-dependencies` led with a noun, the `copy-dist-*` pair put the platform last where every sibling puts it second, and the Swift test pair named a package/library distinction that does not exist — both run the same two test targets, differing only in where they execute. `local-android-library` becomes `publish-android-library-local` rather than `build-*` because it writes to `~/.m2`, the only target here with a side effect outside the working tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`test-e2e` meant web only while reading as a superset of the iOS and Android E2E targets, and the Android trio elided a module slot that meant "library" in `test-android` but "app" in `test-android-e2e` — the same empty slot with two meanings. Marking both slots makes the module axis visible on every platform and exposes a real gap: there is no iOS counterpart to the Android library instrumented tests. Buildkite already keyed the web step `test-web-e2e` while invoking `make test-e2e`, so the pipeline and the Makefile now agree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The grammar constrained name shape but was silent on verb vocabulary and on the `wp-env-*` family, which inverts it by putting the scope first. Both gaps made judgment calls look arbitrary: `check-` reads as a lone outlier next to `lint-`, and the wp-env prefix as an unexplained exception. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It was the only `wp-env-*` target without one, and the only name that had to explain two behaviors — which is why its help text was the longest line in `make help`. `config` covers both paths after the fashion of `git config`: bare invocation reports the current remap, `MODE=on|off` sets it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"Prep" could mean extract, compile, or generate; the script does none of those — it downloads locale bundles from translate.wordpress.org and caches them under src/translations/. Renames the make target, the npm script, and bin/prep-translations.js together so the three keep matching, which is the reason this was left out of the earlier naming pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Step labels determine GitHub check names, and the trunk ruleset requires several by name, so the label changes made alongside the target renames left two required checks unreported. Labels and keys move to a dedicated PR paired with the ruleset update; this one keeps only the `make` invocations that must follow the renamed targets. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The help description already settled on "E2E" over "instrumented" for this target, but its section header still said "Instrumented Tests". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dcalhoun
force-pushed
the
refactor/makefile-target-naming
branch
from
September 21, 2026 18:31
72543b8 to
f1c683f
Compare
Depending on `build` made `make preview` copy the bundle into the tracked iOS resources and Android assets on a fresh clone, and raced `npm ci` under `make -j`. `vite preview` only serves `dist/`, and already reports a missing build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The checks matched hard-coded target names, so a rename that missed one would silently stop the target from forcing a refresh. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three targets repeated its copy inline, leaving four places to keep in sync with the assets path. The iOS targets already call copy-ios-dist. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The translations log still said "Preparing" from the old target name, the E2E help text promised a prompt that CI and existing installs skip, and the wp-env docs omitted that cleaning deletes cached credentials. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Scope mixed languages and platforms, so iOS library tests were `swift` while iOS app tests were `ios`. Naming the platform first wherever a verb spans platforms gives one axis, matching how CI groups its steps. The named sub-slots only fixed word order, which the example already shows. `build` was cited as a true superset but builds only the web bundle, so it is now stated as the one exception. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`lint`, `test`, and `build` span platforms, so their targets now lead with it: `lint-web`, `lint-ios`, `test-web-unit`, `test-ios-library-*`, and `build-ios-*`. `test-web-unit` marks its kind now that `test-web-e2e` competes for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`test-android-library` ran only the JVM unit tests beside `test-android-library-e2e`, so it read as every library test while skipping the E2E suite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`install-deps` and `install-e2e-deps` named the base install by nothing and the E2E one by purpose. `install-web-deps` and `install-web-e2e-deps` read as a pair, and the latter matches `test-web-e2e`, its only user. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`dev-server` and `dev-tools` had no verb, and `dev` also served as the `-dev` mode suffix. `serve` and `serve-dev` now mirror `test-ios-app-e2e` and its `-dev` variant: unmarked means the production build, `-dev` the Vite dev server. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
oguzkocer
approved these changes
Sep 22, 2026
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
3 participants
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.
What?
Rename Makefile targets to follow a consistent pattern.
Why?
As additional Makefile targets were added over time, there was unintentional drift in the naming pattern.
How?
Update targets to match a
<verb>[-<what>][-<how>]grammar. One exception iswp-env-specific targets, which include a namespace prefix since the targets relate to a specific tool's namespace and wrap its commands.npm-dependencies,e2e-dependenciesinstall-web-deps,install-web-e2e-depslint-js[-fix]lint-web[-fix]lint-swift[-fix]lint-ios[-fix]test-js[-watch]test-web-unit[-watch]test-e2e[-ui]test-web-e2e[-ui]test-swift-packagetest-ios-library-simulatortest-swift-librarytest-ios-library-hosttest-ios-e2e[-dev]test-ios-app-e2e[-dev]test-androidtest-android-library-unittest-android-e2e[-dev]test-android-app-e2e[-dev]build-swift-packagebuild-ios-librarybuild-resources-xcframeworkbuild-ios-resources-xcframeworklocal-android-librarypublish-android-library-localcopy-dist-{ios,android}copy-{ios,android}-distpreviewservedev-server[-force]serve-dev[-force]dev-toolsstart-devtoolsprep-translationsfetch-translationswp-env-android-urlswp-env-config-android-urlsUnchanged:
build,clean,format,release,check-wp-packages,lint-android,test-android-library-e2e, andwp-env-{start,stop,clean}.Testing Instructions
This refactor focuses on renaming. To observe the changes, run
make helpand explore/test various targets.Accessibility Testing Instructions
N/A, no user-facing changes.
Screenshots or screencast
N/A, no user-facing changes.
AI-generated details
Makefile target names had drifted into competing shapes — verb-first (
lint-js), noun-first (npm-dependencies), platform-last (copy-dist-ios), and language-scoped beside platform-scoped (test-swift-packagenext totest-ios-e2e) — with no stated rule to settle new additions.Grammar
A header comment at the top of the
Makefilenow documents<verb>[-<what>][-<how>]:android > library > e2e). When a verb has targets on more than one platform, each starts with its platform (web,ios,android); otherwise it starts with whatever the target acts on (fetch-translations).-fix,-watch,-ui,-dev,-host.buildis the one stated exception: it builds only the web bundle, but every other build target starts from it.check-), and a namespace prefix may precede the verb when targets wrap one external tool (wp-env-startmirrorswp-env start).Renames
npm-dependencies,e2e-dependenciesinstall-web-deps,install-web-e2e-depslint-js[-fix]lint-web[-fix]lint-swift[-fix]lint-ios[-fix]test-js[-watch]test-web-unit[-watch]test-e2e[-ui]test-web-e2e[-ui]test-swift-packagetest-ios-library-simulatortest-swift-librarytest-ios-library-hosttest-ios-e2e[-dev]test-ios-app-e2e[-dev]test-androidtest-android-library-unittest-android-e2e[-dev]test-android-app-e2e[-dev]build-swift-packagebuild-ios-librarybuild-resources-xcframeworkbuild-ios-resources-xcframeworklocal-android-librarypublish-android-library-localcopy-dist-{ios,android}copy-{ios,android}-distpreviewservedev-server[-force]serve-dev[-force]dev-toolsstart-devtoolsprep-translationsfetch-translationswp-env-android-urlswp-env-config-android-urlsUnchanged:
build,clean,format,release,check-wp-packages,lint-android,test-android-library-e2e, andwp-env-{start,stop,clean}.A few renames fix more than shape:
test-swift-{package,library}claimed a subject distinction that does not exist. Both run the same tests and differ only in where they execute (iOS Simulator viaxcodebuildvs. host viaswift test), which is now the-simulator/-hostmode.test-androidran only the library's JVM unit tests, and its elided slot meant library whiletest-android-e2e's meant app.test-android-library-unitandtest-android-app-e2emark both.serveandserve-devreplacepreviewanddev-server, whosedevhad no verb and doubled as the-devmode suffix. They now mirrortest-ios-app-e2eand its-devvariant: unmarked is the production build,-devthe Vite dev server.fetch-translationsrenames the make target, the npm script, andbin/prep-translations.jstogether so the three keep matching.Fixes beyond naming
.PHONY: lint-fix-jsnamed a target that does not exist, solint-js-fixwas never actually declared phony. Dates to task: Improve Makefile #237.buildimplied native compilation,test-androidclaimed all Android tests, andtest-e2e/test-jsboth read as "JavaScript tests".$@.copy-android-distinline; they now call it.wp-env-config-android-urlsoutput now states its purpose and the tradeoff in both directions.Notes for review
.buildkite/pipeline.ymlchanges are limited to themakeinvocations that must follow renamed targets. Step labels and keys are intentionally unchanged: labels become the check names the trunk ruleset requires, so they change in ci: align Buildkite step labels, emoji, and keys #714 alongside a ruleset update.AGENTS.md,CONTRIBUTING.md, the SwiftLint Claude hook, and both demo apps' on-screen dev-server notes.make lint-web-fixandmake lint-ios-fixmake no changes,make test-web-unitpasses (235 tests), and no old target names remain.🤖 Generated with Claude Code