Gate cargo fmt --check in CI (bread-ecosystem and bos-settings)

Breadway commented

2026-09-09 09:09:11 +08:00

Owner

Broken out from ECO-15 (bread-ecosystem audit, 2026-09-05). Verified still open on main as of 2026-09-09.

check.yml in this repo (added by ECO-5) and check.yml in bos-settings (added by BSET-3) both run clippy and tests but deliberately skip cargo fmt --check, because the trees are not fmt clean (about 27 files in bread-ecosystem at the time, more in bos-settings) and a mass reformat mid-review would collide with open PRs. check.yml's own comment says to add the step once the tree has been normalised.

Checklist:

bread-ecosystem: run cargo fmt --all in its own dedicated commit, add that commit SHA to .git-blame-ignore-revs, then add a cargo fmt --all --check step to check.yml.
bos-settings: same (its own cargo fmt --all commit, .git-blame-ignore-revs entry, then gate it in that repo's check.yml).

Best done when few PRs are open in each repo. Low priority: purely about stopping formatting drift from accumulating.

Broken out from ECO-15 (bread-ecosystem audit, 2026-09-05). Verified still open on main as of 2026-09-09. `check.yml` in this repo (added by ECO-5) and `check.yml` in bos-settings (added by BSET-3) both run clippy and tests but deliberately skip `cargo fmt --check`, because the trees are not fmt clean (about 27 files in bread-ecosystem at the time, more in bos-settings) and a mass reformat mid-review would collide with open PRs. `check.yml`'s own comment says to add the step once the tree has been normalised. Checklist: - [ ] bread-ecosystem: run `cargo fmt --all` in its own dedicated commit, add that commit SHA to `.git-blame-ignore-revs`, then add a `cargo fmt --all --check` step to `check.yml`. - [ ] bos-settings: same (its own `cargo fmt --all` commit, `.git-blame-ignore-revs` entry, then gate it in that repo's `check.yml`). Best done when few PRs are open in each repo. Low priority: purely about stopping formatting drift from accumulating.

Breadway added the

enhancement

label

2026-09-09 09:09:11 +08:00

Breadway commented

2026-09-09 20:29:02 +08:00

Author

Owner

Keeping this in todo for now. The issue's own precondition ("Best done when few PRs are open in each repo") is not currently met, and this run just made that worse:

bread-ecosystem: PR #24 (ECO-22, touches bread-theme/src/palette.rs + lib.rs) is open, plus the fix/eco-17-chip-height-token branch. A cargo fmt --all here reformats ~27 files and would conflict with both.
bos-settings: PR #10 (BSET-8, frontend/ only, so less exposed) and the fix/remove-titlebar branch are open. cargo fmt --all there is a larger reformat still.

A mass reformat now would force-rebase every open PR through a whitespace-only merge conflict, which is exactly what the check.yml comment and this issue said to avoid.

Recommended sequencing when it is picked up:

Land or close the open PRs in each repo first (get to a near-empty PR queue).
Per repo, in its own dedicated commit: cargo fmt --all, then git rev-parse HEAD >> .git-blame-ignore-revs for that commit, then add the cargo fmt --all --check step to that repo's check.yml (uncommenting the note already there).
Two separate PRs (one per repo) since the change is inherently two-repo; link both to this key.

Low priority and purely about stopping drift, so no urgency to force it against open work.

Keeping this in todo for now. The issue's own precondition ("Best done when few PRs are open in each repo") is not currently met, and this run just made that worse: - **bread-ecosystem**: PR #24 (ECO-22, touches `bread-theme/src/palette.rs` + `lib.rs`) is open, plus the `fix/eco-17-chip-height-token` branch. A `cargo fmt --all` here reformats ~27 files and would conflict with both. - **bos-settings**: PR #10 (BSET-8, `frontend/` only, so less exposed) and the `fix/remove-titlebar` branch are open. `cargo fmt --all` there is a larger reformat still. A mass reformat now would force-rebase every open PR through a whitespace-only merge conflict, which is exactly what the `check.yml` comment and this issue said to avoid. Recommended sequencing when it is picked up: 1. Land or close the open PRs in each repo first (get to a near-empty PR queue). 2. Per repo, in its own dedicated commit: `cargo fmt --all`, then `git rev-parse HEAD >> .git-blame-ignore-revs` for that commit, then add the `cargo fmt --all --check` step to that repo's `check.yml` (uncommenting the note already there). 3. Two separate PRs (one per repo) since the change is inherently two-repo; link both to this key. Low priority and purely about stopping drift, so no urgency to force it against open work.

Breadway commented

2026-09-11 10:52:52 +08:00

Author

Owner

Checked again for this run. Precondition is closer to met now: bread-ecosystem has only one open PR (#24, feature/eco-22-write-wal-cache, palette.rs + lib.rs only) and the fix/eco-17-chip-height-token branch mentioned in the previous comment appears merged and gone from origin. bos-settings similarly has just one open PR (#10, frontend only).

However, the local ~/Projects/bread-ecosystem checkout is currently on a stale local branch (fix/eco-17-chip-height-token) with an untracked .freebuff/ directory, not on main and not clean. Per my working-tree safety rule I don't touch a checkout I didn't leave clean, so I'm skipping this pass rather than resetting someone else's state.

Leaving in todo. Next run: if that checkout is back on a clean main, this is ready to pick up per the sequencing already recommended (fmt commit + .git-blame-ignore-revs entry + check.yml gate, one PR per repo).

Checked again for this run. Precondition is closer to met now: bread-ecosystem has only one open PR (#24, feature/eco-22-write-wal-cache, palette.rs + lib.rs only) and the fix/eco-17-chip-height-token branch mentioned in the previous comment appears merged and gone from origin. bos-settings similarly has just one open PR (#10, frontend only). However, the local ~/Projects/bread-ecosystem checkout is currently on a stale local branch (fix/eco-17-chip-height-token) with an untracked .freebuff/ directory, not on main and not clean. Per my working-tree safety rule I don't touch a checkout I didn't leave clean, so I'm skipping this pass rather than resetting someone else's state. Leaving in todo. Next run: if that checkout is back on a clean main, this is ready to pick up per the sequencing already recommended (fmt commit + .git-blame-ignore-revs entry + check.yml gate, one PR per repo).

Breadway commented

2026-09-11 17:50:54 +08:00

Author

Owner

Both halves implemented, each as its own PR since it's inherently a two-repo change:

bread-ecosystem: #26 (cargo fmt --all in a dedicated commit, .git-blame-ignore-revs added, fmt --check gated in check.yml)
bos-settings: Breadway/bos-settings#12 (same pattern)

Both verified locally: fmt --check, clippy --all-targets -D warnings (bread-ecosystem also with --features gtk), and the full test suite (bread-ecosystem also with --features gtk), all clean before and after.

Precondition was met this run: bread-ecosystem had only PR #25 open (a 1-line unrelated gitignore change, no overlap), bos-settings had zero open PRs.

Both halves implemented, each as its own PR since it's inherently a two-repo change: - bread-ecosystem: https://git.breadway.dev/Breadway/bread-ecosystem/pulls/26 (cargo fmt --all in a dedicated commit, .git-blame-ignore-revs added, fmt --check gated in check.yml) - bos-settings: https://git.breadway.dev/Breadway/bos-settings/pulls/12 (same pattern) Both verified locally: fmt --check, clippy --all-targets -D warnings (bread-ecosystem also with --features gtk), and the full test suite (bread-ecosystem also with --features gtk), all clean before and after. Precondition was met this run: bread-ecosystem had only PR #25 open (a 1-line unrelated gitignore change, no overlap), bos-settings had zero open PRs.

Breadway commented

2026-09-11 18:46:42 +08:00

Author

Owner

Reviewed both halves. Acceptance criteria per the checklist: for each repo, a dedicated cargo fmt --all commit, that commit's SHA recorded in .git-blame-ignore-revs, and a cargo fmt --all --check step added to check.yml. Both PRs satisfy this exactly, each as two commits (style: cargo fmt --all, then ci: gate cargo fmt --check), and I spot-checked the larger reformatted files (bread-theme/src/shell/manifest.rs, bos-settings src/src/commands/bread.rs) to confirm the fmt commits are whitespace-only with no logic changes.

bos-settings half (PR #12): merged (squash, branch deleted). CI was green and the repo is not release-sensitive.

bread-ecosystem half (PR #26): CI green, mergeable, looks ready to merge. Leaving this one to you since bread-ecosystem is on the release train. No action needed beyond a normal merge when you're ready.

Leaving this issue in review since the bread-ecosystem half is still open.

Reviewed both halves. Acceptance criteria per the checklist: for each repo, a dedicated cargo fmt --all commit, that commit's SHA recorded in .git-blame-ignore-revs, and a cargo fmt --all --check step added to check.yml. Both PRs satisfy this exactly, each as two commits (style: cargo fmt --all, then ci: gate cargo fmt --check), and I spot-checked the larger reformatted files (bread-theme/src/shell/manifest.rs, bos-settings src/src/commands/bread.rs) to confirm the fmt commits are whitespace-only with no logic changes. bos-settings half (PR #12): merged (squash, branch deleted). CI was green and the repo is not release-sensitive. bread-ecosystem half (PR #26): CI green, mergeable, looks ready to merge. Leaving this one to you since bread-ecosystem is on the release train. No action needed beyond a normal merge when you're ready. Leaving this issue in review since the bread-ecosystem half is still open.

Breadway closed this issue

2026-09-11 18:49:02 +08:00

Rows
Columns

Gate cargo fmt --check in CI (bread-ecosystem and bos-settings) #21