Skip to content

Commit 253d290

Browse files
paolinopaolino
authored
chore(fmt): pin formatters, enforce in CI, ban drive-by reformatting (#5264)
## Summary - Pin `nixfmt-rfc-style` and `cabal-fmt` in the dev shell so every contributor and every CI job use the same version of every formatter. - Add `just fmt` (runs `fourmolu` + `cabal-fmt` + `nixfmt`) and `just check-fmt` (runs the CI script locally). - Extend `scripts/ci/check-code-format.sh` so CI actually enforces `nixfmt` on every tracked `*.nix` file. - One-time sweep: normalise all `*.nix` with `nixfmt 1.1.0` so the tree matches what CI now enforces. No semantic changes. - Document the rule in `CONTRIBUTING.md` and `docs/.../coding-standards.md`: **no drive-by reformatting.** A style change and a semantic change must not share a commit, and formatter switches are out of scope for feature/fix PRs. ## Why PR #5258 re-formatted `flake.nix` unilaterally with `alejandra`, tucking the formatter swap inside a 7-line feature. That pattern — mixing style and semantics, plus changing formatter with no repo-wide decision — makes diffs unreviewable and `git blame` useless. This PR closes the loophole: one canonical formatter per language, pinned, CI-enforced, documented. ## Commits 1. `style: normalise all *.nix files with nixfmt 1.1.0` — the one-time sweep (1198/954 lines, formatter-only). 2. `chore(nix): pin formatters in dev shell, add just fmt, enforce nixfmt in CI`. 3. `docs(contributing): pin formatters, ban drive-by reformatting`. ## Test plan - [x] `just check-fmt` passes on a clean tree. - [x] `just fmt` is a no-op on a clean tree. - [x] Each commit individually evaluates the flake (`nix eval .#cardano-wallet.name`). - [x] CI green on PR.
2 parents 3d6a4fd + 457c116 commit 253d290

33 files changed

Lines changed: 1276 additions & 954 deletions

‎CONTRIBUTING.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,16 @@ When creating a pull request, please make sure that your code adheres
2121
to our [coding
2222
standards](https://github.com/input-output-hk/adrestia/blob/master/docs/code/Coding-Standards.md).
2323

24+
**Formatting.** Haskell, Cabal and Nix files have canonical formatters
25+
(`fourmolu`, `cabal-fmt`, `nixfmt`) pinned in the dev shell and
26+
enforced by CI. Run `just fmt` before every push.
27+
28+
**No drive-by reformatting.** A style change and a semantic change
29+
must not share a commit, and typically must not share a PR. Pull
30+
requests that reformat files beyond what they meaningfully change —
31+
or that switch formatters — will be rejected. See the [coding
32+
standards](https://cardano-foundation.github.io/cardano-wallet/contributor/what/coding-standards)
33+
for details.
34+
2435
For more information, please consult our [Contributor
2536
Manual](https://cardano-foundation.github.io/cardano-wallet/contributor).

‎docs/site/src/contributor/what/coding-standards.md‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ Each proposal should start with a section justifying the standard with rational
2424
- [Use only a single blank line between top-level definitions](#use-only-a-single-blank-line-between-top-level-definitions)
2525
- [Avoid Variable-Length Indentation](#avoid-variable-length-indentation)
2626
- [Fourmolu is used for code formatting](#fourmolu-is-used-for-code-formatting)
27+
- [Nixfmt is used for \*.nix formatting](#nixfmt-is-used-for-nix-formatting)
28+
- [No drive-by reformatting](#no-drive-by-reformatting)
29+
- [Run every formatter with `just fmt`](#run-every-formatter-with-just-fmt)
2730
- [Haskell Practices](#haskell-practices)
2831
- [Favor `newtype` and tagged type over type-aliases](#favor-newtype-and-tagged-type-over-type-aliases)
2932
- [Language extensions are specified on top of each module](#language-extensions-are-specified-on-top-of-each-module)
@@ -357,6 +360,44 @@ column-limit: 70
357360
```
358361
</details>
359362

363+
### Nixfmt is used for \*.nix formatting
364+
365+
All `*.nix` files in this repository are formatted with
366+
[`nixfmt`](https://github.com/NixOS/nixfmt) (RFC style). The binary is
367+
pinned via `nix/haskell.nix` so every contributor and every CI job use
368+
the exact same version. The format is checked in CI by
369+
`scripts/ci/check-code-format.sh`.
370+
371+
There is no secondary formatter. Do not introduce
372+
[`alejandra`](https://github.com/kamadorueda/alejandra),
373+
`nixpkgs-fmt`, or any other tool.
374+
375+
### No drive-by reformatting
376+
377+
Pull requests that reformat files unrelated to their stated purpose
378+
will be rejected. A semantic change and a style change must never
379+
share a commit — and typically must not share a PR.
380+
381+
> **Why**
382+
>
383+
> Mixed diffs destroy reviewability, pollute `git blame`, and make
384+
> reverts unsafe. When the whole tree is already formatted to a
385+
> canonical style, a drive-by reformat is pure noise.
386+
387+
If you believe the canonical formatter itself should change, open an
388+
issue first. Do not change formatters as part of a feature or fix PR.
389+
390+
### Run every formatter with `just fmt`
391+
392+
```
393+
just fmt # format *.hs, *.cabal and *.nix in place
394+
just check-fmt # run the exact CI format check locally
395+
```
396+
397+
Both recipes rely on tools pinned in the dev shell (`nix develop`);
398+
running them outside the shell will use whatever happens to be on your
399+
`$PATH` and is not guaranteed to match CI.
400+
360401
<details>
361402
<summary>See import grouping example</summary>
362403

‎flake.nix‎

Lines changed: 72 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@
126126
mithril = {
127127
url = "github:input-output-hk/mithril?ref=2603.1";
128128
inputs.nixpkgs.follows = "nixpkgs-unstable";
129-
};
129+
};
130130
};
131131

132132
outputs =
@@ -169,16 +169,23 @@
169169
{
170170
packages =
171171
{ }
172-
// pkgs.lib.optionalAttrs (pkgs.stdenv.hostPlatform.isDarwin && !pkgs.stdenv.cc.nativeLibc && options.packages ? crypton-x509-system) {
173-
# Workaround for broken nixpkgs darwin.security_tool in
174-
# Mojave. This mirrors the workaround in nixpkgs
175-
# haskellPackages.
176-
#
177-
# ref:
178-
# https://github.com/NixOS/nixpkgs/pull/47676
179-
# https://github.com/NixOS/nixpkgs/issues/45042
180-
crypton-x509-system.components.library.preBuild = "substituteInPlace System/X509/MacOS.hs --replace security /usr/bin/security";
181-
};
172+
//
173+
pkgs.lib.optionalAttrs
174+
(
175+
pkgs.stdenv.hostPlatform.isDarwin
176+
&& !pkgs.stdenv.cc.nativeLibc
177+
&& options.packages ? crypton-x509-system
178+
)
179+
{
180+
# Workaround for broken nixpkgs darwin.security_tool in
181+
# Mojave. This mirrors the workaround in nixpkgs
182+
# haskellPackages.
183+
#
184+
# ref:
185+
# https://github.com/NixOS/nixpkgs/pull/47676
186+
# https://github.com/NixOS/nixpkgs/issues/45042
187+
crypton-x509-system.components.library.preBuild = "substituteInPlace System/X509/MacOS.hs --replace security /usr/bin/security";
188+
};
182189
};
183190
in
184191
{
@@ -281,7 +288,10 @@
281288
backend = self.cardano-node;
282289
};
283290
# Local test cluster and mock metadata server
284-
inherit (project.hsPkgs.cardano-wallet.components.exes) mock-token-metadata-server wallet-key-export;
291+
inherit (project.hsPkgs.cardano-wallet.components.exes)
292+
mock-token-metadata-server
293+
wallet-key-export
294+
;
285295
inherit (project.hsPkgs.cardano-wallet-benchmarks.components.exes) benchmark-history;
286296
inherit (project.hsPkgs.local-cluster.components.exes) local-cluster;
287297
integration-exe = project.hsPkgs.cardano-wallet-integration.components.exes.integration-exe;
@@ -306,19 +316,23 @@
306316
unit-cardano-wallet-unit = project.hsPkgs.cardano-wallet-unit.components.tests.unit;
307317
unit-cardano-wallet-primitive = project.hsPkgs.cardano-wallet-primitive.components.tests.test;
308318
unit-cardano-wallet-secrets = project.hsPkgs.cardano-wallet-secrets.components.tests.test;
309-
unit-cardano-wallet-network-layer = project.hsPkgs.cardano-wallet-network-layer.components.tests.unit;
319+
unit-cardano-wallet-network-layer =
320+
project.hsPkgs.cardano-wallet-network-layer.components.tests.unit;
310321
unit-cardano-wallet-test-utils = project.hsPkgs.cardano-wallet-test-utils.components.tests.unit;
311322
unit-cardano-wallet-launcher = project.hsPkgs.cardano-wallet-launcher.components.tests.unit;
312-
unit-cardano-wallet-application-tls = project.hsPkgs.cardano-wallet-application-tls.components.tests.unit;
323+
unit-cardano-wallet-application-tls =
324+
project.hsPkgs.cardano-wallet-application-tls.components.tests.unit;
313325
unit-cardano-numeric = project.hsPkgs.cardano-numeric.components.tests.unit;
314-
unit-cardano-wallet-blackbox-benchmarks = project.hsPkgs.cardano-wallet-blackbox-benchmarks.components.tests.unit;
326+
unit-cardano-wallet-blackbox-benchmarks =
327+
project.hsPkgs.cardano-wallet-blackbox-benchmarks.components.tests.unit;
315328
unit-delta-chain = project.hsPkgs.delta-chain.components.tests.unit;
316329
unit-delta-store = project.hsPkgs.delta-store.components.tests.unit;
317330
unit-delta-table = project.hsPkgs.delta-table.components.tests.unit;
318331
unit-delta-types = project.hsPkgs.delta-types.components.tests.unit;
319332
unit-std-gen-seed = project.hsPkgs.std-gen-seed.components.tests.unit;
320333
unit-wai-middleware-logging = project.hsPkgs.wai-middleware-logging.components.tests.unit;
321-
unit-benchmark-history = project.hsPkgs.cardano-wallet-benchmarks.components.tests.benchmark-history-test;
334+
unit-benchmark-history =
335+
project.hsPkgs.cardano-wallet-benchmarks.components.tests.benchmark-history-test;
322336
wallet-key-export-test = project.hsPkgs.cardano-wallet.components.tests.wallet-key-export-test;
323337

324338
# Combined project coverage report
@@ -446,47 +460,50 @@
446460
format = "zip";
447461
};
448462
# Per-test-exe bundles for Windows CI.
449-
tests = let
450-
mkTest = name: test:
451-
import ./nix/windows-test-exe.nix {
452-
inherit pkgs test name;
463+
tests =
464+
let
465+
mkTest =
466+
name: test:
467+
import ./nix/windows-test-exe.nix {
468+
inherit pkgs test name;
469+
};
470+
in
471+
{
472+
wallet-unit = import ./nix/windows-test-exe.nix {
473+
inherit pkgs;
474+
name = "wallet-unit";
475+
test = windowsPackages.unit-cardano-wallet-unit;
476+
extraPkgs = [ windowsPackages.cardano-cli ];
477+
testDataDirs = [
478+
./lib/unit/test/data
479+
./lib/local-cluster/test/data
480+
];
453481
};
454-
in {
455-
wallet-unit = import ./nix/windows-test-exe.nix {
456-
inherit pkgs;
457-
name = "wallet-unit";
458-
test = windowsPackages.unit-cardano-wallet-unit;
459-
extraPkgs = [windowsPackages.cardano-cli];
460-
testDataDirs = [
461-
./lib/unit/test/data
462-
./lib/local-cluster/test/data
463-
];
464-
};
465-
wallet-primitive = import ./nix/windows-test-exe.nix {
466-
inherit pkgs;
467-
name = "wallet-primitive";
468-
test = windowsPackages.unit-cardano-wallet-primitive;
469-
testDataDirs = [./lib/primitive/test/data];
470-
};
471-
wallet-secrets = mkTest "wallet-secrets" windowsPackages.unit-cardano-wallet-secrets;
472-
wallet-network-layer = mkTest "wallet-network-layer" windowsPackages.unit-cardano-wallet-network-layer;
473-
wallet-test-utils = mkTest "wallet-test-utils" windowsPackages.unit-cardano-wallet-test-utils;
474-
wallet-launcher = mkTest "wallet-launcher" windowsPackages.unit-cardano-wallet-launcher;
475-
wallet-application-tls = mkTest "wallet-application-tls" windowsPackages.unit-cardano-wallet-application-tls;
476-
cardano-numeric = mkTest "cardano-numeric" windowsPackages.unit-cardano-numeric;
477-
wallet-blackbox-benchmarks = import ./nix/windows-test-exe.nix {
478-
inherit pkgs;
479-
name = "wallet-blackbox-benchmarks";
480-
test = windowsPackages.unit-cardano-wallet-blackbox-benchmarks;
481-
testDataDirs = [./lib/wallet-benchmarks/test/data];
482+
wallet-primitive = import ./nix/windows-test-exe.nix {
483+
inherit pkgs;
484+
name = "wallet-primitive";
485+
test = windowsPackages.unit-cardano-wallet-primitive;
486+
testDataDirs = [ ./lib/primitive/test/data ];
487+
};
488+
wallet-secrets = mkTest "wallet-secrets" windowsPackages.unit-cardano-wallet-secrets;
489+
wallet-network-layer = mkTest "wallet-network-layer" windowsPackages.unit-cardano-wallet-network-layer;
490+
wallet-test-utils = mkTest "wallet-test-utils" windowsPackages.unit-cardano-wallet-test-utils;
491+
wallet-launcher = mkTest "wallet-launcher" windowsPackages.unit-cardano-wallet-launcher;
492+
wallet-application-tls = mkTest "wallet-application-tls" windowsPackages.unit-cardano-wallet-application-tls;
493+
cardano-numeric = mkTest "cardano-numeric" windowsPackages.unit-cardano-numeric;
494+
wallet-blackbox-benchmarks = import ./nix/windows-test-exe.nix {
495+
inherit pkgs;
496+
name = "wallet-blackbox-benchmarks";
497+
test = windowsPackages.unit-cardano-wallet-blackbox-benchmarks;
498+
testDataDirs = [ ./lib/wallet-benchmarks/test/data ];
499+
};
500+
delta-chain = mkTest "delta-chain" windowsPackages.unit-delta-chain;
501+
delta-store = mkTest "delta-store" windowsPackages.unit-delta-store;
502+
delta-table = mkTest "delta-table" windowsPackages.unit-delta-table;
503+
delta-types = mkTest "delta-types" windowsPackages.unit-delta-types;
504+
std-gen-seed = mkTest "std-gen-seed" windowsPackages.unit-std-gen-seed;
505+
wai-middleware-logging = mkTest "wai-middleware-logging" windowsPackages.unit-wai-middleware-logging;
482506
};
483-
delta-chain = mkTest "delta-chain" windowsPackages.unit-delta-chain;
484-
delta-store = mkTest "delta-store" windowsPackages.unit-delta-store;
485-
delta-table = mkTest "delta-table" windowsPackages.unit-delta-table;
486-
delta-types = mkTest "delta-types" windowsPackages.unit-delta-types;
487-
std-gen-seed = mkTest "std-gen-seed" windowsPackages.unit-std-gen-seed;
488-
wai-middleware-logging = mkTest "wai-middleware-logging" windowsPackages.unit-wai-middleware-logging;
489-
};
490507
e2e = import ./nix/windows-test-exe.nix {
491508
inherit pkgs;
492509
name = "e2e";

‎justfile‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,21 @@ default:
99
syntax:
1010
scripts/ci/check-code-format.sh
1111

12+
# format every Haskell, Cabal and Nix file the CI format check covers
13+
fmt:
14+
#!/usr/bin/env bash
15+
set -euo pipefail
16+
echo "+++ fourmolu"
17+
fourmolu --mode inplace $(git ls-files -- '*.hs')
18+
echo "+++ cabal-fmt"
19+
find lib -name '*.cabal' -exec cabal-fmt -i {} \;
20+
echo "+++ nixfmt"
21+
nixfmt $(git ls-files -- '*.nix')
22+
23+
# verify formatting matches CI (runs the exact CI script)
24+
check-fmt:
25+
scripts/ci/check-code-format.sh
26+
1227
hlint:
1328
nix develop --command bash -c 'hlint lib'
1429

‎nix/config.nix‎

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,32 @@
1-
lib: customConfig: let
2-
config =
3-
lib.recursiveUpdate {
4-
platform = "all";
5-
# The systems that the jobset will be built for.
6-
supportedSystems = import ./nix/supported-systems.nix;
1+
lib: customConfig:
2+
let
3+
config = lib.recursiveUpdate {
4+
platform = "all";
5+
# The systems that the jobset will be built for.
6+
supportedSystems = import ./nix/supported-systems.nix;
77

8-
# Enable debug tracing
9-
debug = false;
8+
# Enable debug tracing
9+
debug = false;
1010

11-
# Use this git revision for stamping executables
12-
gitrev = null;
11+
# Use this git revision for stamping executables
12+
gitrev = null;
1313

14-
# Enable building the cabal-cache util - only needed under CI
15-
withCabalCache = false;
14+
# Enable building the cabal-cache util - only needed under CI
15+
withCabalCache = false;
1616

17-
# optional extra haskell.nix module
18-
haskellNix = {};
17+
# optional extra haskell.nix module
18+
haskellNix = { };
1919

20-
# optional string argument to override compiler, in cabal shell.
21-
ghcVersion = null;
20+
# optional string argument to override compiler, in cabal shell.
21+
ghcVersion = null;
2222

23-
dockerHubRepoName = null;
24-
}
25-
customConfig;
23+
dockerHubRepoName = null;
24+
} customConfig;
2625
in
27-
assert lib.asserts.assertOneOf "platform" config.platform
28-
["all" "linux" "macos" "windows"]; config
26+
assert lib.asserts.assertOneOf "platform" config.platform [
27+
"all"
28+
"linux"
29+
"macos"
30+
"windows"
31+
];
32+
config

‎nix/default.nix‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
{...} @ args:
1+
{ ... }@args:
22
with (import ./flake-compat.nix args);
3-
defaultNix.legacyPackages.${builtins.currentSystem}.pkgs
3+
defaultNix.legacyPackages.${builtins.currentSystem}.pkgs

0 commit comments

Comments
 (0)