Skip to content

Commit

Permalink
Make CI catch formatting errors
Browse files Browse the repository at this point in the history
Previously we used the `makeFormatRule` on CI, which would format all
files regardless instead of reporting errors when that file was not
formatted correctly.

This commit adds a `--ci` command line argument to `./configure.mjs`
that reports formatting errors instead of attempting to fix them.

Additionally we (in a slightly more convoluted manner than I would like)
fix the `TODO` on checking that the top-level `package.json` is
formatted correctly.

One nice benefit of the `--ci` change is increased performance.  When we
format our files in-place we do not run `npm ci` on any of the plugins
until we have first formatted their `package.json` - which is blocked by
installing `biome` at the top level.  With `--ci` we have the format
check as a validation step so we can run `npm ci` in parallel across all
plugins immediately and then run the validation only when `biome` is
installed at the root.  This saves about 5s locally since `npm ci` is
relatively slow.
  • Loading branch information
elliotgoodrich committed Mar 23, 2024
1 parent b2acec2 commit 5d21aaf
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 18 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,5 @@ jobs:
cache: 'npm'
- uses: seanmiddleditch/gha-setup-ninja@master
- run: npm ci
- run: npm run configure
- run: npm run configure -- --ci
- run: ninja -k 0
49 changes: 37 additions & 12 deletions configure.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ import {
} from "@ninjutsu-build/core";
import { makeTSCRule, makeTypeCheckRule } from "@ninjutsu-build/tsc";
import { makeNodeTestRule } from "@ninjutsu-build/node";
import { makeFormatRule, makeLintRule } from "@ninjutsu-build/biome";
import {
makeCheckFormattedRule,
makeFormatRule,
makeLintRule,
} from "@ninjutsu-build/biome";
import { makeTranspileRule } from "@ninjutsu-build/bun";
import { basename, dirname, extname, relative, join } from "node:path/posix";
import { readFileSync, writeFileSync } from "node:fs";
Expand All @@ -19,6 +23,7 @@ const touch = platform() == "win32" ? "type NUL > $out" : "touch $out";
const prefix = platform() === "win32" ? "cmd /c " : "";

const useBun = process.argv.includes("--bun");
const inCi = process.argv.includes("--ci");

function makeNpmCiRule(ninja) {
const ci = ninja.rule("npmci", {
Expand Down Expand Up @@ -152,7 +157,29 @@ const ninja = new NinjaBuilder({
ninja.output += "\n";
ninja.comment("Rules + Installation");
const ci = makeNpmCiRule(ninja);
const toolsInstalled = ci({ in: "package.json" });

// We would like to check whether `package.json` is formatted correctly.
// Most of the rules inject a build-order dependency on `npm ci` having
// run correctly, but we also need a validation dependency from running
// `npm ci` so we have a cycle (in JS only, ninja is happy with a cycle
// containing a validations edge). This means it's a bit convoluted to
// create the `checkFormatted` rule but that what the code below does.
let checkFormatted;

const toolsInstalled = ci({
in: "package.json",
[validations]: (out) => {
checkFormatted = addBiomeConfig(
inject(makeCheckFormattedRule(ninja), {
[orderOnlyDeps]: out,
}),
"biome.json",
);
// Add a validation that `package.json` is formatted correctly.
// If we formatted after running `npmci` it would cause us to run it again
return checkFormatted({ in: "package.json" })[validations];
},
});

const link = makeNpmLinkRule(ninja);
const tsc = inject(makeTSCRule(ninja), { [orderOnlyDeps]: toolsInstalled });
Expand All @@ -161,12 +188,14 @@ const typecheck = inject(makeTypeCheckRule(ninja), {
});
const test = makeNodeTestRule(ninja);
const tar = makeTarRule(ninja);
const format = addBiomeConfig(
inject(makeFormatRule(ninja), {
[orderOnlyDeps]: toolsInstalled,
}),
"biome.json",
);
const format = inCi
? checkFormatted
: addBiomeConfig(
inject(makeFormatRule(ninja), {
[orderOnlyDeps]: toolsInstalled,
}),
"biome.json",
);
const lint = addBiomeConfig(
inject(makeLintRule(ninja), {
[orderOnlyDeps]: toolsInstalled,
Expand All @@ -183,10 +212,6 @@ const transpileArgs = useBun

const { phony } = ninja;

// TODO: Add a validation that `package.json` is formatted correctly.
// We need to format after running `npmci` but then formatting the
// file will cause us to rerun `npmci` again

format({ in: "configure.mjs", cwd: "." });

const scope = "@ninjutsu-build/";
Expand Down
8 changes: 4 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"license": "MIT",
"devDependencies": {
"@biomejs/biome": "1.4.0",
"@ninjutsu-build/biome": "^0.8.0",
"@ninjutsu-build/biome": "^0.8.1",
"@ninjutsu-build/bun": "^0.1.0",
"@ninjutsu-build/core": "^0.8.5",
"@ninjutsu-build/node": "^0.8.0",
Expand Down

0 comments on commit 5d21aaf

Please sign in to comment.