From 093b1b21cf6ccd928dbec37f02ca3a722e4431e1 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 6 Nov 2026 15:00:49 +0100 Subject: [PATCH] docs: document the commit rules and their hook --- tests/test_doc_pages.py | 49 +++++++++++++++++++++++++++++++++++++++++ wiki/testing-guide.md | 17 ++++++++++++-- wiki/tools.md | 5 +++-- 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/tests/test_doc_pages.py b/tests/test_doc_pages.py index bc7a6a5a7..aa691ec21 100644 --- a/tests/test_doc_pages.py +++ b/tests/test_doc_pages.py @@ -2050,3 +2050,52 @@ class BuildYourOwnPackSaysWhatTheCloneCosts(unittest.TestCase): def test_advanced_usage_names_the_shallow_clone(self): self.assertIn("--depth 1", _page("advanced-usage.md").split("## Custom packs")[0]) + + +class LandingAChangeNamesTheCommitRules(unittest.TestCase): + """The page left out the review-tag rule scripts/commit_rules.py + enforces, and never said where the rules live or how a clone runs them: + the hook is not versioned.""" + + def _landing(self) -> str: + return _section(_page("testing-guide.md"), "Landing a change") + + def test_every_refusal_is_on_the_page(self): + import commit_rules + + section = self._landing() + words = re.search(r"\\b\((.*?)\)", commit_rules.REFUSED_WORDS.pattern).group(1) + from tests.test_prose_style import NoMarketingVocabulary + + for word in words.replace("|", " ").split(): + if NoMarketingVocabulary.WORDS.search(word): + # Named by "the marketing vocabulary above": spelled out, + # the page would fail that test. + continue + with self.subTest(word=word): + self.assertIn(word, section.lower()) + self.assertIn("which review asked for it", section) + self.assertIn("scripts/commit_rules.py", section) + + def test_the_hook_command_on_the_page_refuses_a_review_tag(self): + import subprocess + import tempfile + + block = re.search(r"```bash\n(.*?)```", self._landing(), re.S).group(1) + with tempfile.TemporaryDirectory(dir=ROOT / "tmp") as scratch: + clone = Path(scratch) + (clone / ".git" / "hooks").mkdir(parents=True) + (clone / "scripts").mkdir() + (clone / "scripts" / "commit_rules.py").write_bytes( + (ROOT / "scripts" / "commit_rules.py").read_bytes()) + subprocess.run(["sh", "-c", block], cwd=clone, check=True) + message = clone / "MSG" + message.write_text("fix: address review 3 notes\n", encoding="utf-8") + refused = subprocess.run([str(clone / ".git" / "hooks" / "commit-msg"), "MSG"], + cwd=clone, capture_output=True, text=True) + message.write_text("fix: say what the change does\n", encoding="utf-8") + accepted = subprocess.run([str(clone / ".git" / "hooks" / "commit-msg"), "MSG"], + cwd=clone, capture_output=True, text=True) + self.assertEqual(refused.returncode, 1, refused.stderr) + self.assertIn("which review asked for it", refused.stderr) + self.assertEqual(accepted.returncode, 0, accepted.stderr) diff --git a/wiki/testing-guide.md b/wiki/testing-guide.md index a9f94702d..ef18126b2 100644 --- a/wiki/testing-guide.md +++ b/wiki/testing-guide.md @@ -373,6 +373,9 @@ to the way the project writes: line naming a .info, core-info or System.dat no "is correct", "is accurate", "still declares", "omits" or "missing". The note states what that file declares and what the code reads. +- No pointer at a review in a comment, a docstring or a test name: a review + number in parentheses, a note file named after one, a test module or class + named after a review. The comment says what the code does. Formatting stays: bold labels, lists, tables and admonitions make a page easier to use. When markup shows raw on the site, the rendering is fixed and @@ -456,8 +459,10 @@ A commit subject is English, 50 characters at most, `type: description` with a lowercase word after the colon. The types are `feat`, `refactor`, `chore`, `docs` and `fix`. The message does not use bug, regression, revert, dead code, broken, wrong, hack, workaround or implement (derived forms included), nor the -marketing vocabulary above, and carries no Co-Authored-By trailer. A body -explains the why when the subject cannot: the scenario that failed, the +marketing vocabulary above, and carries no Co-Authored-By trailer. It says +what the change does, never which review asked for it: "review 3", "audit 12" +or "round 5" with its number points at notes no clone holds and is refused. A +body explains the why when the subject cannot: the scenario that failed, the upstream `file:line` that decides it, what users will see. ``` @@ -465,6 +470,14 @@ feat: add retrobat platform support refactor: extract shared utils to common.py ``` +`scripts/commit_rules.py` holds these rules. Git does not copy hooks with a +clone, so the commit-msg hook that runs them is set up once per clone: + +```bash +printf '#!/bin/sh\nexec python3 scripts/commit_rules.py "$1"\n' > .git/hooks/commit-msg +chmod +x .git/hooks/commit-msg +``` + Files are staged by name, never with `git add -A` or `git add .`, which sweep in deletions and work that is not yours. Generated artifacts (`database.json`, `retired.json`, `derivable.json`, `install/*.json`, `README.md`, `CONTRIBUTING.md`, `exports/`, diff --git a/wiki/tools.md b/wiki/tools.md index ce8c14359..6e0024392 100644 --- a/wiki/tools.md +++ b/wiki/tools.md @@ -814,8 +814,9 @@ hard-links `.cache/large/`, `data/` and the gitignored `bios/` files from the main checkout. Before each job it is fast-forwarded, or rebased when it holds unpushed commits, onto `origin/main`. A job that finds anything uncommitted there stops as `blocked` and cleans nothing. Commits name their files -(`git commit -- `), and their subjects are checked against the -commit-msg hook when the registry loads. A job that leaves a change no commit +(`git commit -- `), and their subjects are checked against +`scripts/commit_rules.py`, the rules the commit-msg hook runs, when the +registry loads. A job that leaves a change no commit rule takes fails. `--push` sends the batch: rebase on `origin/main` (a conflict keeps the batch