Align contributor documentation with the repository, and check that it stays aligned - #3389
Open
Crypto2099 wants to merge 4 commits into
Open
Align contributor documentation with the repository, and check that it stays aligned#3389Crypto2099 wants to merge 4 commits into
Crypto2099 wants to merge 4 commits into
Conversation
… exist The checklist required a thread on a Slack workspace this project cannot reach, two GitHub teams that do not exist, a named individual reviewer, and a Jira ticket, and it asked for `yarn manage:translations`, which is now `yarn i18n:manage`. It also required a CHANGELOG entry, which 20 of the last 25 merged pull requests did not add. A checklist whose items cannot be satisfied trains reviewers to tick without reading, which devalues the items that are real. The replacement names the exact local command for every CI check, so a reviewer reproduces what the build will do rather than asserting it from memory. The code quality items carry over unchanged, and a regression-test line replaces the acceptance-test items that referenced a test system this project does not operate.
Sync failures are explained by where the blockchain is stored and on what kind of volume, rather than by the steps the user took. Hardware wallet failures depend on four components that version independently: Daedalus, the device firmware, the Cardano app, and the operating system. A general bug form asks for none of this, so those reports arrive needing a round trip before triage can begin. Both forms open with the diagnostic information from Help > Daedalus Diagnostics, and both gate submission on acknowledging that wallet secrets are excluded and that the report is public. The device list is the set Daedalus supports, from `source/common/types/hardware-wallets.types.ts`.
Each of these was checked against the tree rather than assumed. `AGENTS.md` and `.agent/readme.md` both stated Electron 24.2.0; `package.json` resolves 41.3.0. The same tables named `*.test.tsx` as the test convention, where the tree has 73 `*.spec.ts` files and no `*.test.tsx`. `.agent/readme.md` linked the i18n-messaging and theme-management skills under `skills/frontend/`, which is not where they live, and omitted `thermo-nuclear-code-quality-review` from the skills table. `.agent/workflows/frontend.md` carried the same two paths. `.agent/skills/theme-management/SKILL.md` had four links one directory level too high, all resolving above the repository root. `.agent/workflows/frontend.md` showed `useContext(IntlContext)`, a symbol that appears nowhere in the tree and an API that postdates the pinned react-intl 2.9.0. The sample now uses `injectIntl`, which is how the 50 function components that need `intl` obtain it. The SCSS sample used `composes:`, which appears zero times in `source/`. `.agent/workflows/test.md` documented pre-commit and pre-push hooks. There are no git hooks in this repository and neither husky nor lefthook is a dependency, so nothing ran on commit or push for any reader who relied on it. `.agent/system/state-management.md` gave a `setupStores(api, actions)` signature that has never existed; it is `setUpStores` and takes four parameters. `tests/README.md` documented `yarn test:e2e:byron` and a Byron-specific section, where no `@byron` scenarios and no such script exist, and directed readers to `yarn nix:dev`, which is not one of the four `nix:` shells. `installers/README.md` pointed at `default.nix` and `cardano-sl-src.json` at the repository root, neither of which is present; the node and wallet are pinned as flake inputs.
Documentation here drifts because nothing can fail. Every correction in the preceding commit was mechanically decidable and none of it was decided, because no check looked: a renamed directory left two skill links dangling, a renamed script stayed in the pull request template, and a restated Electron version fell 17 major releases behind the dependency it named. The check makes four assertions over tracked markdown. Every relative link resolves. Every `yarn` command named inside backticks is a script in `package.json`, a yarn subcommand, or a listed exception carrying its reason. References to systems this project no longer uses do not reappear. Versions restated in prose match what `package.json` resolves. `.agent/plans/` and `CHANGELOG.md` are excluded. Both are records of what was true when written, correct as history and wrong as present-tense instruction. Asserting over them would either fail permanently or require editing the record to keep a build green. It runs as `nix build .#checks.x86_64-linux.docs`, joining the static analysis set that already runs on one system because the result is identical everywhere.
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.
Summary
Brings the contributor-facing documentation in line with the repository as it actually is, and adds a check so the next divergence fails a build instead of misleading a reader.
Every correction below was verified against the tree rather than assumed.
Tracking: se7en-labs-inc#14, whose first two tasks ship in #3387. The two forms here cover the report categories that a general bug form serves worst.
The pull request template
The checklist required a thread on a Slack workspace this project cannot reach, two GitHub teams that do not exist, a named individual reviewer, and a Jira ticket. It asked for
yarn manage:translations, which is nowyarn i18n:manage, and a CHANGELOG entry that 20 of the last 25 merged pull requests did not add.The replacement names the exact local command for every CI check, so a reviewer reproduces what the build will do rather than asserting it from memory. The code quality items carry over unchanged.
Issue forms for sync and hardware wallet problems
Sync failures are explained by where the blockchain is stored and on what kind of volume, not by the steps the user took. Hardware wallet failures depend on four components that version independently: Daedalus, the device firmware, the Cardano app, and the operating system. A general bug form asks for none of that, so those reports arrive needing a round trip before triage can begin.
Both forms open with the diagnostic information from
Help>Daedalus Diagnostics. That instruction assumes the copy control in #3384; until it lands there is nothing for a reporter to click, so the two want to land together or close to it.The device list is the set Daedalus supports, from
source/common/types/hardware-wallets.types.ts.Corrections to the guidance set
AGENTS.mdand.agent/readme.md: Electron 24.2.0package.jsonresolves 41.3.0*.test.tsx*.spec.ts, zero*.test.tsxskills/frontend/skills/theme-management/SKILL.md: four linksfrontend.md:useContext(IntlContext)frontend.md:composes:in SCSSsource/test.md: pre-commit and pre-push hooksstate-management.md:setupStores(api, actions)setUpStores, four parameterstests/README.md:yarn test:e2e:byron,yarn nix:dev@byronscenariosinstallers/README.md:default.nix,cardano-sl-src.jsonThe skills table also omitted
thermo-nuclear-code-quality-review.The check
Documentation drifts here because nothing can fail. Each correction above was mechanically decidable and none of it was decided, because no check looked.
nix build .#checks.x86_64-linux.docsmakes four assertions over tracked markdown:yarncommand named inside backticks is a script inpackage.json, a yarn subcommand, or a listed exception carrying its reason.package.jsonresolves..agent/plans/andCHANGELOG.mdare excluded. Both are records of what was true when written, correct as history and wrong as present-tense instruction. Asserting over them would either fail permanently or require editing the record to keep a build green.BESTPRACTICES.mdis exempt from the third assertion, with the reason recorded in the check. Its Git chapter prescribes adevelopbranch and YouTrack identifiers; retiring that document is a decision in its own right, and half-editing a file that may be going away is worse than naming the exemption.Verification
nix build .#checks.x86_64-linux.docspasses over 51 files. Reintroducing each historical failure was confirmed to fail it:nix fmtreports no changes. Both issue forms parse as GitHub issue form schemas.