docs: make the path from clone to merged PR followable - #65
Merged
Conversation
The diagram in `lib.rs` and the core README read
scanner ──► index ──► dedup ──► graph ──► rules + risk ──► report
and two parts of that were wrong. Dedup now runs *after* classification,
because a duplicate set is an offer to act and entries nothing will act
on are excluded from it — the order changed when that landed and the
diagram didn't. And there has never been an `index` module; the stage
was aspirational and reads, to someone opening the crate for the first
time, like a file they failed to find.
Both now show the real sequence, with a note on why dedup sits where it
does and a pointer at `report::build_with`, which is the whole thing in
one function.
There was no answer to "I cloned it, now what". The README and
CONTRIBUTING each carried four lines of `cargo` commands that assume a
working toolchain and say nothing about what to install, what CI will
check, or what the failures mean.
docs/DEVELOPMENT.md covers the path end to end: prerequisites per
platform, the fast engine-only loop, the app, the site, how to run what
CI runs, and a table of the failures that actually come up — each
symptom quoted as it really appears.
The two that cost the most time are both invisible from the code. A
machine with `rustc` but no C linker cannot run even `cargo check`, and
says `linker 'cc' not found`. And building the app without the Linux
webview packages fails naming `glib-2.0`, not webkitgtk, because
`glib-sys` is the first `*-sys` crate to fall over — so the error gives
no hint which four packages to install. Both messages were reproduced
rather than recalled.
Also documents the two test-writing traps this workspace has: a fixture
under a temp directory matches the shipped temp-file rules on both Linux
and Windows, so report tests need their own small `RulesDb`; and
`contains("a/b")` finds nothing on Windows, which is exactly how the
cross-platform matrix drew blood on its first run.
It opened with four cargo commands and a list of checks, which answers "how do I run the checks" and nothing else. Someone arriving with time to spare still had no idea what was worth doing. Now it leads with the honest answer: rules. The shipped database covers five applications, everything else on a disk classifies as unknown and is dropped, and adding a rule needs no Rust at all — so the highest -value contribution is also the lowest barrier, and that ought to be the first thing a newcomer reads. Also written down, because they are currently learned by having a PR turned down: a wrong `safe` verdict is not a cosmetic bug, since both `safe` and `review` are actionable in the app; and capability belongs in the engine, with the frontends rendering what it returns. Plus commit and PR conventions taken from what this repo actually does, and a table of which file a given intent belongs in. The checklist now matches CI exactly — including `--all-targets`, and a note that `--workspace` drags in the desktop app and its system dependencies, which is a confusing first failure to hit.
The doc explained the workspace layout and the design constraints, and stopped short of the thing a newcomer needs most: what actually happens between "user picks a folder" and "a report comes back", and which file each step lives in. Added that walkthrough — five stages, each naming the function that does the work and why it sits where it does — plus a section on choosing the layer for a new feature, which is the decision most likely to be got wrong and most expensive to unpick in review. The recurring answer is "further down than you think": a rule beats code, and the engine beats a frontend.
Four places describing behaviour that has since changed. The CLI's sample output showed one reason per finding, from before it printed all of them — so the example omitted exactly the evidence that makes a verdict make sense. It now shows a risky row with its `referenced by 3 projects` line, which is the clearest illustration of what the impact graph is for. The app README listed no commands at all, though the webview can now call six. Added them with a note on which mutate, since that is the question worth answering there. The FAQ said the desktop app targets Windows, macOS and Linux. It builds on all three and CI compiles it on all three, but only Linux and Windows release — the macOS entry is commented out pending a developer account, so claiming a macOS release was a promise nothing keeps. It also described quarantine as restorable without mentioning the manifest that now makes that survive a restart, which is the part users would want to know. Plus the README and docs index pointing at the new setup guide.
"Chrome, Firefox, pip, Rust and Node build output, and a handful of system directories" left out logs, crash dumps, installers and temp directories, which is a third of the database. Counted and named instead — the point of the paragraph is how small the set is, so the number should be right.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Someone who clones this repo today gets four
cargolines in the READMEand four more in CONTRIBUTING, both of which assume a working toolchain
and neither of which says what to install, what to work on, or what the
failures mean. This makes the path from
git cloneto a merged PRsomething a person can actually follow.
New: docs/DEVELOPMENT.md
Prerequisites per platform, the fast engine-only loop, the app, the
site, how to run exactly what CI runs, and a table of failures with the
message as it really appears. Two of those cost real time and are
invisible from the code:
rustcbut no C linker can't run evencargo check,and says
linker 'cc' not found.glib-2.0, not webkitgtk, becauseglib-sysis the first*-syscrate to fall over — so the error gives no hint which four packages
you actually need.
Both were reproduced rather than recalled. There's also a container
recipe for machines where a C compiler can't be installed at all, with
the Podman/Docker file-ownership difference noted, since that one leaves
root-owned files in your checkout.
Rewritten: CONTRIBUTING.md
Leads with the honest answer to "what should I work on": rules. The
database is thirteen rules, everything else on a disk classifies as
unknownand is dropped, and adding one needs no Rust — so thehighest-value contribution is also the lowest barrier, and that should
be the first thing a newcomer reads.
Also written down, because they are currently learned by having a PR
turned down: a wrong
safeverdict isn't cosmetic, sincesafeandrevieware both actionable in the app; and capability belongs in theengine with the frontends rendering what it returns. Plus commit and PR
conventions taken from what this repo actually does, a table of which
file a given intent belongs in, and a checklist that matches CI exactly.
Extended: docs/ARCHITECTURE.md
A scan walked through the engine stage by stage, each step naming the
function that does the work and why it sits where it does — plus a
section on choosing the layer for a new feature, which is the decision
most likely to be got wrong and most expensive to unpick in review.
Corrections
Documentation that had drifted from the code:
lib.rsand the core README drewscanner ─► index ─► dedup ─► graph ─► rules + risk ─► report. Dedupmoved after classification when non-actionable entries were excluded
from it, and there has never been an
indexmodule — that stage reads,to someone opening the crate, like a file they failed to find.
printed all of them, so the example omitted exactly the evidence that
makes a verdict make sense.
six.
on macOS, but the release entry is commented out pending a developer
account, so that was a promise nothing keeps.
Verified, not assumed
Every command and error message in the new guide was run. Beyond that:
typos(the exact version CI uses) passesbase.json, not rememberedChecklist
cargo fmt --allandcargo clippy --workspaceare cleancargo test --workspacepassesfix in
lib.rs; no behaviour change