Automattic / Automattic/jetpack-crm

Gate the release build on lint-css without breaking the shared Makefile

Open
#17 1 comment 0 reactions 0 assignees View on GitHub
enhancement ready-for-human
Dominant language
PHP
Stars
13
Forks
8
Avg merge
1d 10h
Merged PRs (30d)
13

Description

Follow-up to #14.

## Background

#14 added stylelint with two `@wordpress/theme` rules (`no-unknown-ds-tokens`, `no-token-fallback-values`) and removed all 25 hardcoded `var()` fallbacks from the emerald Sass partials. It runs via `make lint-css` or `npm run lint:css`.

Nothing invokes it automatically, so today it only catches something if someone remembers to run it. That is weak in a specific way: the trigger for this class of breakage is not editing CSS, it is bumping `@wordpress/theme`. #13 broke because a rename landed under Sass nobody touched. "I'm not editing CSS so I don't need the CSS linter" is exactly the wrong inference at the one moment the check would earn its keep.

Now that the fallbacks are gone there is no safety net either. An unknown token renders as an unset property rather than quietly degrading to a stale hex.

## The obvious fix, and why it was pulled from #14

Make `lint-css` a prerequisite of `build`:

```make
build: lint-css ## Build dist/zero-bs-crm.zip ...
./scripts/build-plugin.sh
```

`make build` is the only entry point to `scripts/build-plugin.sh` (`scripts/create-release.mjs:203`), so this gates every release. I had it working in #14: a bad token aborts with `make: *** [lint-css] Error 2` and `build-plugin.sh` never runs.

I took it back out. The `Makefile` is parameterised (`PLUGIN_NAME := zero-bs-crm`) and copied around the plugin family alongside the release scripts. Adding a CRM-only prerequisite to a common target means `make build` breaks in any of those repos that has no stylelint config, no `lint:css` script, or no stylelint installed.

## What needs deciding

A portable form. The naive conditional swallows real failures, which defeats the point:

```make
@test -f stylelint.config.mjs && npm run lint:css || true
```

That passes when the config is missing *and* when the lint genuinely fails. Some shape that skips cleanly when the tooling is absent but still fails hard when it is present and unhappy. Options worth weighing:

- Guard on the config file existing, but keep the exit status when it does.
- Put the hook in `build-plugin.sh` instead, which is already repo-specific and not shared.
- A generic `lint-assets` target in the shared Makefile that is a no-op by default and each repo fills in.

The second is probably the least intrusive, since `build-plugin.sh` is explicitly listed as not-shared. Worth a look before committing to the Makefile route.

Whatever wins should be backported to the family per the shared-tooling rule, or deliberately scoped to CRM only.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with the shared Makefile build target, scripts/build-plugin.sh, scripts/create-release.mjs, and the stylelint configuration and npm script described in the issue. Compare the proposed hook locations and verify behavior when lint tooling is absent, when lint passes, and when it fails. Done means release builds are gated where appropriate without breaking repositories that share the Makefile.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, shell
Domain
build-system, release
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.