From ea45507ac82d9356f3e9a475fe9140ec525e378c Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 14 Oct 2026 16:35:03 +0000 Subject: [PATCH] :wrench: Raise commit message limits to 80 chars Relax the subject limit from 70 to 80 characters and the body wrap limit from 76 to 80. Merge/Revert/Reapply commits skip the subject length check since git prepends those prefixes to an existing subject. Update the checker, its tests, and all related docs and memories accordingly. AI-assisted-by: muse-spark-1.3-contributor Signed-off-by: Andrey Antukh --- .agents/skills/create-commit/SKILL.md | 7 +-- .serena/memories/critical-info.md | 2 +- .serena/memories/workflow/creating-commits.md | 18 ++++---- .serena/memories/workflow/creating-prs.md | 2 +- AGENTS.md | 3 +- CONTRIBUTING.md | 8 ++-- scripts/check-commit | 29 ++++++++----- scripts/test_check_commit.py | 43 +++++++++++++------ 8 files changed, 71 insertions(+), 41 deletions(-) diff --git a/.agents/skills/create-commit/SKILL.md b/.agents/skills/create-commit/SKILL.md index bf0667ca65..dd85191fd9 100644 --- a/.agents/skills/create-commit/SKILL.md +++ b/.agents/skills/create-commit/SKILL.md @@ -23,10 +23,11 @@ subject/body limits, and the `AI-assisted-by` trailer. Follow it exactly. ## Iron Rules (non-negotiable) -1. **Wrap every body line at 76 characters or fewer.** Count characters, do +1. **Wrap every body line at 80 characters or fewer.** Count characters, do not eyeball. Exceptions: `Signed-off-by:` / `AI-assisted-by:` trailers and lines carrying a URL. This is the rule agents skip most often. -2. **Subject ≤70 chars**, imperative, capitalized, no trailing period. +2. **Subject ≤80 chars**, imperative, capitalized, no trailing period. + `Merge`/`Revert`/`Reapply` commits skip the length check. 3. **Blank line between subject and body.** 4. **Run `./scripts/check-commit` and require exit code 0.** It mechanically checks rules 1–3. A non-zero exit is a hard blocker: fix the message and @@ -41,7 +42,7 @@ subject/body limits, and the `AI-assisted-by` trailer. Follow it exactly. that does not match the stated intent, **STOP** and tell the user before committing. 3. Draft the message following the format in the memory doc, wrapping the body - at 76 characters per line, and run: + at 80 characters per line, and run: ```bash git commit -m "" -m "" ``` diff --git a/.serena/memories/critical-info.md b/.serena/memories/critical-info.md index 5e84c9834e..d0304e1fe8 100644 --- a/.serena/memories/critical-info.md +++ b/.serena/memories/critical-info.md @@ -12,7 +12,7 @@ You are working on the GitHub project `penpot/penpot`, a monorepo. # Development workflow - Commit/PR/issue creation is **on explicit request only**. Before any of these actions, read the relevant memory — don't infer format from prior examples: - - Before `git commit` → `mem:workflow/creating-commits` (subject/body format, 76-char body wrapping enforced by `scripts/check-commit`, `AI-assisted-by: model-name` trailer) + - Before `git commit` → `mem:workflow/creating-commits` (subject/body format, 80-char body wrapping enforced by `scripts/check-commit`, `AI-assisted-by: model-name` trailer) - Before `gh issue create` → `mem:workflow/creating-issues` (title derivation, body template, labels, Issue Type) - Before `gh pr create` / `gh pr edit` → `mem:workflow/creating-prs` (title format, body structure, "Note:" line) - Before a repo-wide pnpm version update → `mem:workflow/updating-pnpm` (workspace layout, `scripts/sync-pnpm-version` flow, the stamp-missing-field and ignored-builds gotchas, verification steps) diff --git a/.serena/memories/workflow/creating-commits.md b/.serena/memories/workflow/creating-commits.md index a60234819f..678681d4ba 100644 --- a/.serena/memories/workflow/creating-commits.md +++ b/.serena/memories/workflow/creating-commits.md @@ -11,12 +11,11 @@ automatically pull the identity from the local git config `user.name` and `user. ## Message Format ``` -:emoji: Subject line (imperative, capitalized, no period, <=70 chars) +:emoji: Subject line (imperative, capitalized, no period, <=80 chars) Body explaining what changed and why. -Wrap lines at 76 characters — git log adds a -four-space indent, so 76 + 4 fits an 80-column -terminal. Keep each line concise. +Wrap lines at 80 characters. +Keep each line concise. AI-assisted-by: model-name ``` @@ -26,16 +25,17 @@ AI-assisted-by: model-name These rules are not advisory. Do not commit until every one holds. A commit that breaks them is wrong, even if the code is right. -- **Body lines MUST wrap at 76 characters or fewer.** Measure every line; do - not eyeball it. This is the rule most often skipped. Rationale: `git log` - indents the body four spaces, so 76 + 4 fits an 80-column terminal. -- **Subject MUST be ≤70 chars**, imperative, capitalized, no trailing period. +- **Body lines MUST wrap at 80 characters or fewer.** Measure every line; do + not eyeball it. This is the rule most often skipped. +- **Subject MUST be ≤80 chars**, imperative, capitalized, no trailing period. + Merge/Revert/Reapply commits skip the length check: git prepends those + prefixes to an existing subject. - **MUST be a blank line** between subject and body. - **MUST run `scripts/check-commit` and get exit code 0 before finishing.** It mechanically validates the rules above; a failing run is a blocker. - It checks `HEAD` by default: `./scripts/check-commit` - For another commit: `./scripts/check-commit -c ` -- **NEVER** hand-wave the body as "one long line". If a line exceeds 76, +- **NEVER** hand-wave the body as "one long line". If a line exceeds 80, break it at a space. - **NEVER add a `Co-authored-by:` trailer.** It is forbidden; `scripts/check-commit` fails the commit when any line matches it (case-insensitive). - CI runs `scripts/check-commit` on every new commit (`mem:workflow/creating-commits` is enforced by `.github/workflows/commit-checker.yml`), so a forbidden trailer fails the build. diff --git a/.serena/memories/workflow/creating-prs.md b/.serena/memories/workflow/creating-prs.md index f9b41275ef..4d9d0fe8d4 100644 --- a/.serena/memories/workflow/creating-prs.md +++ b/.serena/memories/workflow/creating-prs.md @@ -27,7 +27,7 @@ Always add the PR to the Main project (`--project "Main"`) unless the user expli PR titles follow commit title conventions: ``` -:emoji: Subject line (imperative, capitalized, no period, <=70 chars) +:emoji: Subject line (imperative, capitalized, no period, <=80 chars) ``` See `mem:workflow/creating-commits` for emoji codes. Squash merge uses the PR title as the final commit subject, so title format matters. diff --git a/AGENTS.md b/AGENTS.md index fa95b4c045..e900e32518 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -17,7 +17,8 @@ - **Read `AGENTS.local.md` after this file whenever it exists.** It carries the developer's steering for this project and overrides this file on prose style and workflow, but not on project policy, where instructions here prevail. -- **Commit message body lines MUST wrap at ≤76 chars** (subject ≤70 chars) and +- **Commit message body lines MUST wrap at ≤80 chars** (subject ≤80 chars, + except `Merge`/`Revert`/`Reapply` which skip the subject length check) and the commit MUST pass `./scripts/check-commit` with exit code 0 before you consider it done. This is mechanically checked — do not eyeball it. - **Read the workflow memory BEFORE the corresponding action**: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2d665b0715..9b23ae0fa9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -124,7 +124,7 @@ commit history — what matters is the [PR title](#title). Pull request titles **must** follow the same convention as commit subjects: ``` -:emoji: Subject line (imperative, capitalized, no period, <=70 chars) +:emoji: Subject line (imperative, capitalized, no period, <=80 chars) ``` Read [Creating Commits](./.serena/memories/workflow/creating-commits.md) @@ -220,8 +220,10 @@ Commit messages must follow this format: - Capitalize the first letter of the subject - Add clear and concise description on the body - Do not end the subject with a period -- Keep the subject to **70 characters** or fewer -- **Wrap body lines at 76 characters or fewer** (trailers and URLs excepted) +- Keep the subject to **80 characters** or fewer + (`Merge`/`Revert`/`Reapply` commits skip this limit, since git prepends + those prefixes to an existing subject) +- **Wrap body lines at 80 characters or fewer** (trailers and URLs excepted) - Separate the subject from the body with a **blank line** - Never add a `Co-authored-by` trailer — it is forbidden and fails `./scripts/check-commit` diff --git a/scripts/check-commit b/scripts/check-commit index d0e9f59899..df4672ea59 100755 --- a/scripts/check-commit +++ b/scripts/check-commit @@ -5,7 +5,7 @@ Check commit messages against Penpot's commit guidelines. Validates commit messages using the rules defined in: - .github/workflows/commit-checker.yml (regex pattern) - CONTRIBUTING.md (formatting rules, subject length, DCO) - - .serena/memories/workflow/creating-commits.md (body wrapped at 76 chars) + - .serena/memories/workflow/creating-commits.md (body wrapped at 80 chars) By default, checks HEAD. Use --commit to specify a different commit. @@ -39,12 +39,17 @@ COMMIT_PATTERN = re.compile( MERGE_PATTERN = re.compile(r"^(Merge|Revert|Reapply).+[^.]$") +# ── Subject line length ────────────────────────────────────────────────────── +# Subject lines must fit in 80 characters. Merge/Revert/Reapply commits are +# exempt: git prepends "Merge ..."/"Revert ..." to an existing subject, so +# enforcing the limit would fail on text the author did not write. +MAX_SUBJECT_LINE = 80 + # ── Body line wrapping ─────────────────────────────────────────────────────── -# Commit bodies must wrap at 76 characters (see -# .serena/memories/workflow/creating-commits.md). That leaves room for the -# four-space indent git log adds, fitting an 80-column terminal. Trailers and +# Commit bodies must wrap at 80 characters (see +# .serena/memories/workflow/creating-commits.md). Trailers and # URLs are exempt: they cannot be wrapped without losing meaning. -MAX_BODY_LINE = 76 +MAX_BODY_LINE = 80 TRAILER_PATTERN = re.compile( r"^(Signed-off-by|Co-developed-by|Reviewed-by|" @@ -110,11 +115,13 @@ def check_regex(message): def check_subject_length(message): - """Subject line must be ≤ 70 characters.""" + """Subject line must be ≤ 80 characters (Merge/Revert/Reapply exempt).""" first_line = message.split("\n")[0] - if len(first_line) > 70: + if MERGE_PATTERN.match(first_line): + return True, None + if len(first_line) > MAX_SUBJECT_LINE: return False, ( - f"Subject line exceeds 70 characters ({len(first_line)} chars):\n" + f"Subject line exceeds {MAX_SUBJECT_LINE} characters ({len(first_line)} chars):\n" f" {first_line}" ) return True, None @@ -166,7 +173,7 @@ def check_body_blank_line(message): def check_body_line_length(message): - """Body lines must wrap at 76 characters or fewer. + """Body lines must wrap at 80 characters or fewer. The subject (first line) has its own length rule. Trailers (e.g. Signed-off-by) and lines carrying a URL are exempt, since wrapping them @@ -250,11 +257,11 @@ def main(): validators = [ ("Regex pattern", check_regex), - ("Subject ≤ 70 chars", check_subject_length), + ("Subject ≤ 80 chars", check_subject_length), ("No trailing period in subject", check_subject_no_trailing_dot), ("Subject capitalized", check_subject_capitalized), ("Blank line after subject", check_body_blank_line), - ("Body wrapped at 76 chars", check_body_line_length), + ("Body wrapped at 80 chars", check_body_line_length), ("No Co-authored-by trailer", check_no_co_authored_by), ] diff --git a/scripts/test_check_commit.py b/scripts/test_check_commit.py index a3a7a1c2b9..037bc8fd56 100644 --- a/scripts/test_check_commit.py +++ b/scripts/test_check_commit.py @@ -55,20 +55,20 @@ class BodyLineLengthTests(unittest.TestCase): self.assert_ok(message) def test_line_at_limit_passes(self): - line = "x " * 38 # 76 chars, breakable - self.assertEqual(len(line), 76) + line = "x " * 40 # 80 chars, breakable + self.assertEqual(len(line), 80) self.assert_ok(":bug: Fix crash\n\n" + line + "\n") def test_line_one_over_limit_fails(self): - line = "x " * 38 + "x" # 77 chars, breakable - self.assertEqual(len(line), 77) + line = "x " * 40 + "x" # 81 chars, breakable + self.assertEqual(len(line), 81) error = self.assert_fail(":bug: Fix crash\n\n" + line + "\n") - self.assertIn("76", error) + self.assertIn("80", error) def test_long_body_line_fails(self): long_line = "word " * 20 # 100 chars, breakable error = self.assert_fail(":bug: Fix crash\n\n" + long_line + "\n") - self.assertIn("76", error) + self.assertIn("80", error) self.assertIn("line 3", error) def test_subject_is_not_checked(self): @@ -150,15 +150,34 @@ class SubjectRulesRegressionTests(unittest.TestCase): ok, _ = check_commit.check_subject_no_trailing_dot(":bug: Fix crash.") self.assertFalse(ok) - def test_subject_at_70_chars_passes(self): - # ":bug: " is 6 chars, so 64 chars of text reach exactly 70. - ok, error = check_commit.check_subject_length(":bug: " + "S" * 64) + def test_subject_at_80_chars_passes(self): + # ":bug: " is 6 chars, so 74 chars of text reach exactly 80. + ok, error = check_commit.check_subject_length(":bug: " + "S" * 74) self.assertTrue(ok, error) - def test_subject_over_70_chars_fails(self): - ok, error = check_commit.check_subject_length(":bug: " + "S" * 65) + def test_subject_over_80_chars_fails(self): + ok, error = check_commit.check_subject_length(":bug: " + "S" * 75) self.assertFalse(ok) - self.assertIn("70", error) + self.assertIn("80", error) + + def test_merge_subject_skips_length_check(self): + # Git generates Merge subjects from branch names; the limit is skipped. + subject = "Merge branch 'some-very-long-branch-name-that-keeps-going' into develop with extra text here" + self.assertGreater(len(subject), 80) + ok, error = check_commit.check_subject_length(subject) + self.assertTrue(ok, error) + + def test_revert_subject_skips_length_check(self): + subject = 'Revert ":bug: Fix some earlier commit with a fairly long subject line already here" extra' + self.assertGreater(len(subject), 80) + ok, error = check_commit.check_subject_length(subject) + self.assertTrue(ok, error) + + def test_reapply_subject_skips_length_check(self): + subject = 'Reapply ":bug: Fix some earlier commit with a fairly long subject line already here" extra' + self.assertGreater(len(subject), 80) + ok, error = check_commit.check_subject_length(subject) + self.assertTrue(ok, error) if __name__ == "__main__":