mirror of
https://github.com/penpot/penpot.git
synced 2026-10-28 21:40:34 +00:00
🔧 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 <niwi@niwi.nz>
This commit is contained in:
parent
9f598fca5b
commit
ea45507ac8
@ -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 "<subject>" -m "<body>"
|
||||
```
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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 <ref>`
|
||||
- **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.
|
||||
|
||||
@ -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.
|
||||
|
||||
@ -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**:
|
||||
|
||||
@ -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`
|
||||
|
||||
@ -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),
|
||||
]
|
||||
|
||||
|
||||
@ -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__":
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user