fabi/Sylpheed
1
0
Fork 0
Code Issues 9 Pull Requests Actions Packages Projects Releases Wiki Activity

CI — the tree has never been rustfmt-clean, and reformatting it now would collide with two long-lived branches #12

New Issue
Closed
opened 2026-09-04 19:14:05 +00:00 by fabi · 1 comment
fabi commented 2026-09-04 19:14:05 +00:00
Owner
Copy Link

cargo fmt --all -- --check reports a ~13,000 line diff across the workspace. Pre-existing; main fails identically; no current PR touches a .rs file.

This one is a scoping decision, not a task, which is why it has no owner label yet.

The obvious fix — run cargo fmt --all and commit — is the dangerous one right now. auto/frame-blend-draw-path is 495 commits ahead of main and auto/port-p6-audio is 366, and both are queued to come back as reviewable PRs (#7, #8). A whole-tree reformat on main before those land turns every one of those merges into a conflict in every file, and the review that #7 and #8 exist to make possible becomes unreadable.

Three options, in the order I would take them:

  1. Defer until the long-lived branches have landed, then reformat once, as its own commit that touches nothing else. Cheapest, and the ordering is the whole point.
  2. Reformat now and accept the conflicts. Only worth it if the branches are being abandoned rather than merged.
  3. Drop the job. Honest if nobody intends to be rustfmt-clean — better than a permanently red check nobody reads.

What is not an option: softening the job so it reports pass while the tree is unformatted. That is the shape PROTOCOL.md now forbids.

`cargo fmt --all -- --check` reports a **~13,000 line diff** across the workspace. Pre-existing; `main` fails identically; no current PR touches a `.rs` file. **This one is a scoping decision, not a task, which is why it has no owner label yet.** The obvious fix — run `cargo fmt --all` and commit — is the dangerous one right now. `auto/frame-blend-draw-path` is **495 commits** ahead of `main` and `auto/port-p6-audio` is **366**, and both are queued to come back as reviewable PRs (#7, #8). A whole-tree reformat on `main` before those land turns every one of those merges into a conflict in every file, and the review that #7 and #8 exist to make possible becomes unreadable. Three options, in the order I would take them: 1. **Defer** until the long-lived branches have landed, then reformat once, as its own commit that touches nothing else. Cheapest, and the ordering is the whole point. 2. **Reformat now** and accept the conflicts. Only worth it if the branches are being abandoned rather than merged. 3. **Drop the job.** Honest if nobody intends to be rustfmt-clean — better than a permanently red check nobody reads. **What is not an option:** softening the job so it reports pass while the tree is unformatted. That is the shape PROTOCOL.md now forbids.
fabi added this to the Infrastructure milestone 2026-09-04 19:14:05 +00:00
fabi added the kind/defectkind/itemstate/proposed labels 2026-09-04 19:14:05 +00:00
fabi commented 2026-09-06 07:32:27 +00:00
Author
Owner
Copy Link

The deferral argument, measured. 81% of this debt has no collision risk at all.

#12 defers on the grounds that reformatting main would collide with auto/frame-blend-draw-path (495 commits) and auto/port-p6-audio (366) and make #7/#8 unreviewable. That is the right shape of argument — it is the one that turned out to be overstated for #13, so it is worth measuring here rather than inheriting.

Taken from run 207's Formatting job (774 hunks, the baseline count) and checked file-by-file against both branches:

files hunks
byte-identical on both long-lived branches 133 627 (81%)
diverge on at least one 21 147

133 files carrying 627 hunks cannot collide with anything, because neither branch has touched them. Reformatting those is free of the risk this issue is deferring on.

File-level is the correct granularity here, unlike #13 — rustfmt rewrites a whole file, so any divergence in that file counts, and no line-level exemption applies.

The 21 that genuinely block

hunks  file                                                    frame-blend / port-p6
   27  crates/sylpheed-cli/src/main.rs                          100 /   0
   12  crates/sylpheed-export/src/screen.rs                     465 /  39
   12  crates/sylpheed-export/src/check.rs                      115 /   0
   12  crates/sylpheed-export/src/audio.rs                     1142 /   0   (deleted on frame-blend)
   12  crates/sylpheed-export/examples/record_loop_control.rs   130 /   0
   10  crates/sylpheed-export/src/main.rs                       314 / 172
    9  crates/sylpheed-formats/tests/slb_leading_segment_disc.rs 59 /   0
    8  crates/sylpheed-export/examples/dialog_pairs.rs           87 /   0
    7  crates/sylpheed-formats/src/ui_layout.rs                 750 /   0
   …   11 more, 2–5 hunks each

Two things stand out. Fourteen of the 21 are in sylpheed-export, which is the crate that pins sylpheed-formats from a git tag rather than the workspace and is largely independent. And export/src/audio.rs is deleted on frame-blend, so its 12 hunks are work that the merge will throw away regardless.

What this changes, and what it does not

It does not make the Formatting job green. 147 hunks would remain, so cargo fmt --all -- --check still exits non-zero, and scoping the job to the clean subset is not on the table — that is the softening shape.

What it changes is the decision available. The options in the body are framed as all-or-nothing over time ("defer until the branches land, then reformat once"). A third is now measurable:

Reformat the 133 uncontested files now, as one commit touching nothing else; leave the 21 until #7/#8 land. That retires 81% of the debt with zero risk to the reviewability this issue exists to protect, shrinks the eventual reformat from 154 files to 21, and — because 14 of those 21 are in one mostly-independent crate — makes the remainder a conversation with two branch owners rather than a whole-tree event.

The cost is honest and should be stated: the tree is then partially formatted, which is a state nobody designed, and the fmt job stays red throughout. If that is judged worse than one clean sweep later, deferring everything is still defensible — but it should be chosen against the real number, which is 21 files, not 154.

Measurement only. Nothing reformatted, no commit, and #12 remains state/proposed.

## The deferral argument, measured. 81% of this debt has no collision risk at all. #12 defers on the grounds that reformatting `main` would collide with `auto/frame-blend-draw-path` (495 commits) and `auto/port-p6-audio` (366) and make #7/#8 unreviewable. That is the right *shape* of argument — it is the one that turned out to be overstated for #13, so it is worth measuring here rather than inheriting. Taken from run 207's Formatting job (774 hunks, the baseline count) and checked file-by-file against both branches: | | files | hunks | |---|---|---| | **byte-identical on both long-lived branches** | **133** | **627 (81%)** | | diverge on at least one | 21 | 147 | **133 files carrying 627 hunks cannot collide with anything**, because neither branch has touched them. Reformatting those is free of the risk this issue is deferring on. File-level is the correct granularity here, unlike #13 — rustfmt rewrites a whole file, so any divergence in that file counts, and no line-level exemption applies. ### The 21 that genuinely block ``` hunks file frame-blend / port-p6 27 crates/sylpheed-cli/src/main.rs 100 / 0 12 crates/sylpheed-export/src/screen.rs 465 / 39 12 crates/sylpheed-export/src/check.rs 115 / 0 12 crates/sylpheed-export/src/audio.rs 1142 / 0 (deleted on frame-blend) 12 crates/sylpheed-export/examples/record_loop_control.rs 130 / 0 10 crates/sylpheed-export/src/main.rs 314 / 172 9 crates/sylpheed-formats/tests/slb_leading_segment_disc.rs 59 / 0 8 crates/sylpheed-export/examples/dialog_pairs.rs 87 / 0 7 crates/sylpheed-formats/src/ui_layout.rs 750 / 0 … 11 more, 2–5 hunks each ``` Two things stand out. **Fourteen of the 21 are in `sylpheed-export`**, which is the crate that pins `sylpheed-formats` from a git tag rather than the workspace and is largely independent. And `export/src/audio.rs` is **deleted** on `frame-blend`, so its 12 hunks are work that the merge will throw away regardless. ### What this changes, and what it does not It does **not** make the Formatting job green. 147 hunks would remain, so `cargo fmt --all -- --check` still exits non-zero, and scoping the job to the clean subset is not on the table — that is the softening shape. What it changes is the decision available. The options in the body are framed as all-or-nothing over time ("defer until the branches land, then reformat once"). A third is now measurable: **Reformat the 133 uncontested files now, as one commit touching nothing else; leave the 21 until #7/#8 land.** That retires 81% of the debt with zero risk to the reviewability this issue exists to protect, shrinks the eventual reformat from 154 files to 21, and — because 14 of those 21 are in one mostly-independent crate — makes the remainder a conversation with two branch owners rather than a whole-tree event. The cost is honest and should be stated: the tree is then *partially* formatted, which is a state nobody designed, and the fmt job stays red throughout. If that is judged worse than one clean sweep later, deferring everything is still defensible — but it should be chosen against the real number, which is 21 files, not 154. Measurement only. Nothing reformatted, no commit, and #12 remains `state/proposed`.
fabi closed this issue 2026-09-08 20:09:53 +00:00
fabi removed the state/proposed label 2026-09-17 19:02:21 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
agent/decoder
Owned by the Decoder (disc to meaning; runs the emulator)
 agent/port
Owned by the Port (disc to playable; no RE)
 kind/ask
One agent asking the other for something it cannot answer in role
 kind/bundle
A bundle the human defined; agents decompose it into items
 kind/defect
Found by a play-test or a check
 kind/item
One unit of work, small enough to finish in a single session
 state/approved
Human approved the shape; an agent may start
 state/blocked
Waiting on another item; use the Depends-On field, not prose
 state/in-progress
An agent is working it now
 state/needs-human
Done as far as an agent can tell -- a person must look. The body says what to look at
 state/proposed
Agent proposed this item; awaiting the human's approval to start
No Label kind/defect kind/item
Milestone
No items
No Milestone Infrastructure
Projects
Clear projects
No project
Assignees
Clear assignees
sylph-decoder sylph-pi sylph-port fabi
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fabi/Sylpheed#12
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?