Skip to content

docs: clarify CLAUDE.md pre-commit gate fallback for GUI-incapable contributors - #457

Open
Jah-yee wants to merge 2 commits into
lacs-project:mainfrom
Jah-yee:fix-claude-md-fallback
Open

Jah-yee wants to merge 2 commits into
lacs-project:mainfrom
Jah-yee:fix-claude-md-fallback

Conversation

@Jah-yee

@Jah-yee Jah-yee commented Sep 19, 2026

Copy link
Copy Markdown

Context

Issue #451: Neither CLAUDE.md nor CONTRIBUTING.md said which pre-commit gate governs when a contributor cannot run the full workspace test suite.

Fix

Added a cross-reference paragraph to the Pre-Commit Gate section of CLAUDE.md:

  • Directs contributors to CONTRIBUTING.md fallback when they lack GUI platform libraries
  • Reminds them to leave evidence files untouched and name the missing dependency in the PR body
  • Maintains the integrity of metadata by preventing hand-edited workarounds

This makes the fallback discoverable at the point where a contributor first encounters the gate, rather than requiring them to read both files in full.

Closes #451

The footer said '_{total} actions have an ActionSpec' but
ListJobHistory is part of KNOWN_ACTION_NAMES even though it has no
ActionSpec and is not in the table. The total catalogue count is
total + 1, so the first sentence must also say total + 1.

Closes lacs-project#455
…ntributors

When a contributor cannot install the GUI platform libraries, they cannot
run `cargo nextest run --workspace`. The CONTRIBUTING.md fallback was
described but neither file said which governs when both apply. This
inserts a cross-reference paragraph in the pre-commit gate section
directing contributors to the CONTRIBUTING.md fallback and reminding them
not to edit evidence files by hand.

Closes lacs-project#451

@vladimirrott vladimirrott left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You put the paragraph where the reader hits the wall, in the Pre-Commit Gate section rather than in a new section further down. That is the right instinct and it is what #451 asks for: a contributor who is blocked reads the rule that blocks them, not a file they have not opened yet.

Requesting changes at 7c87d25e. One hard blocker that is not about your writing, and one gap in scope.

Blocking: the branch carries a commit that does not compile

This PR branches off #456 rather than main, so it ships that change as well:

$ git log --oneline main..p457
7c87d25 docs: clarify CLAUDE.md pre-commit gate fallback for GUI-incapable contributors
d93f42d fix(test): correct action count in action-reference footer

d93f42d fails to build. Rust's inline format arguments take a bare identifier, so {total + 1} is a parse error:

$ out="$(podman run --rm --network=none -v "/tmp/r20p456:/repo:z" -v "$HOME/.cargo:/cargo:O" -w /repo -e CARGO_HOME=/cargo -e CARGO_TARGET_DIR=/repo/.container-target -e CARGO_NET_OFFLINE=true docker.io/library/rust:1-slim cargo test -p sysknife-daemon --test action_reference_doc --offline --no-run 2>&1)"; rc=$?; echo "rc=$rc"; printf '%s\n' "$out" | grep -v '^warning: unused manifest key' | tail -30
rc=101
error: invalid format string: the `+` sign flag must appear after `:` in a format string
   --> crates/sysknife-daemon/tests/action_reference_doc.rs:140:25
    |
140 |         "---\n\n_{total + 1} actions have an `ActionSpec` and are tabled above. The \
    |                         ^ expected `:` before `+` sign flag in format string

Rebasing this branch onto main drops that commit and leaves a one-file documentation change, which is what the title describes. The rest of my reading is about the CLAUDE.md hunk alone.

Your checks had not run at all when I picked this up, because GitHub holds a first-time contributor's fork runs at action_required until a maintainer approves them. I have approved all eight runs across this PR and #456. That delay was mine, not yours.

Scope: the contributor-facing half is still missing

#451 asks for the precedence to be stated in both files, and names CONTRIBUTING.md as the one that matters most, because that is the contributor guide. This PR leaves it untouched, so someone who reads CONTRIBUTING.md and never opens CLAUDE.md is in the same position the issue describes.

The cross-reference also points at a passage that is narrower than your sentence says. CONTRIBUTING.md:149-156 grants its fallback for one artifact, the test-count metadata:

$ sed -n '149,156p' CONTRIBUTING.md
The GUI workspace members still need the platform libraries named in CI. If
you cannot install those, do not estimate a count or edit the evidence metadata
by hand. Leave `tests/evidence/workspace-tests.json` and the three prose files
untouched. In the PR body, name the command you could not run and the missing
tool or system library that blocked it. A maintainer will run the full suite and
regenerate all four files before merge. This is the supported fallback; an
honest missing measurement is better than metadata that describes a run which
never happened.

Nothing there says a contributor may commit without a clean cargo nextest run --workspace --locked, which is the permission your paragraph directs them to. Generalising that passage past the artifact is part of the same fix.

Two more things the issue asks for that would make this land:

  • Say what a contributor runs instead. scripts/ci-local.sh --no-postgres exists and already prints an INCOMPLETE line naming the gate that did not run, so the recommendation is concrete rather than "run what you can".
  • Settle the --exclude sysknife-shell question rather than skipping it. The package name is sysknife-shell, and the issue's one condition is that nobody has confirmed the exclusion produces a clean run on a host without WebKit and GTK. Either test it and write down the result, or leave the flag out. An untested command in the contributor guide is worse than no command.

Optional

Two blank lines land after the new paragraph where one is the house style. CLAUDE.md is not on the markdownlint-cli2 list in .github/workflows/ci.yml:150-158, so nothing will go red over it.

What I would do next

Rebase onto main, then extend the change to CONTRIBUTING.md with the same precedence sentence and the ci-local.sh --no-postgres pointer. That closes #451 outright. If you would rather ship only the CLAUDE.md half now, drop Closes #451 from the body and I will merge it as a partial and leave the issue open for the rest; say which you prefer and I will work to it.

Separately: you picked an issue about contributors whose machine cannot build the GUI crate, which suggests you administer a Linux box rather than only reading about one. If so, the read-only surface is worth twenty minutes. sysknife "check disk usage" and sysknife "which systemd services are failed?" plan and display without executing anything, and the daemon runs nothing you have not approved. Whatever you hit is the kind of report we act on. No obligation, and it has no bearing on this merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nothing says whether CONTRIBUTING.md's fallback or CLAUDE.md's pre-commit gate governs a contributor who cannot build the GUI crate

2 participants