Skip to content

Commit 0266479

Browse files
committed
ci(lint): forbid misleading type+scope combinations in commit messages
Add a custom gitlint rule (UC1) that rejects feat(ci), fix(ci), feat(e2e), and fix(e2e). These combinations pollute user-facing release notes with infrastructure changes that mean nothing to end users. The rule provides actionable error messages pointing to the correct prefix, e.g. "Use ci(<subsystem>)" or "Use ci(e2e)". Signed-off-by: rbean <rbean@redhat.com> Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
1 parent ce66d92 commit 0266479

5 files changed

Lines changed: 151 additions & 0 deletions

File tree

.gitlint

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
[general]
22
# Enable conventional commits title check
33
contrib=CT1
4+
extra-path=gitlint_rules/
45

56
[title-max-length]
67
line-length=100

COMMITS.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,17 @@ Apply the same discipline to `fix` — bumping a dependency version is `chore`,
5050

5151
The parenthesized scope is optional but encouraged. Use it to identify the subsystem: `feat(appsetup)`, `fix(mint)`, `docs(adr)`, `chore(ci)`. When fixing a specific issue, prefer the issue number as scope: `fix(#123): ...`.
5252

53+
### Forbidden type + scope combinations
54+
55+
Some type/scope pairs are **enforced as errors** by gitlint (rule `UC1`). These combinations mislead users by putting infrastructure changes in user-facing release-note sections.
56+
57+
| Forbidden | Why | Use instead |
58+
|---|---|---|
59+
| `fix(ci)` | CI changes are not user-visible bug fixes | `ci(<subsystem>)` |
60+
| `feat(ci)` | CI changes are not user-visible features | `ci(<subsystem>)` |
61+
| `fix(e2e)` | E2E test changes are not user-visible bug fixes | `ci(e2e)` |
62+
| `feat(e2e)` | E2E test changes are not user-visible features | `ci(e2e)` |
63+
5364
## Breaking changes
5465

5566
Breaking changes **must** be marked in both commit messages and PR titles. GoReleaser builds release notes from merged PR titles (`use: github` in `.goreleaser.yml`), so an unmarked PR title means the breaking change is invisible to users reading the release notes. This has caused real incidents — users upgraded with no warning that their agents would stop working.

Makefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ script-test:
129129
$(call run-timed,bash internal/scaffold/fullsend-repo/scripts/pre-fetch-prior-review-test.sh)
130130
$(call run-timed,python3 internal/scaffold/fullsend-repo/scripts/process-fix-result-test.py)
131131
$(call run-timed,python3 skills/topissues/scripts/topissues_test.py)
132+
$(call run-timed,python3 -m pytest gitlint_rules_test.py -v)
132133

133134
test: lint-all go-test script-test lint-eval-cases
134135

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
"""Gitlint rule that forbids misleading type+scope combinations.
2+
3+
Types like ``feat`` and ``fix`` appear in user-facing release notes.
4+
Scopes like ``ci`` and ``e2e`` describe infrastructure, not user-visible
5+
changes. Using them together pollutes the release notes with entries
6+
that mean nothing to end users.
7+
"""
8+
9+
import re
10+
11+
from gitlint.rules import CommitRule, RuleViolation
12+
13+
# Pattern: type(scope): description
14+
_CONVENTIONAL = re.compile(r"^(?P<type>\w+)\((?P<scope>[^)]+)\)")
15+
16+
# Map of (type, scope) -> suggested replacement.
17+
# Scope matches are case-insensitive.
18+
_FORBIDDEN = {
19+
("feat", "ci"): "ci(<subsystem>)",
20+
("fix", "ci"): "ci(<subsystem>)",
21+
("feat", "e2e"): "ci(e2e)",
22+
("fix", "e2e"): "ci(e2e)",
23+
}
24+
25+
26+
class ForbiddenTypeScope(CommitRule):
27+
name = "forbidden-type-scope"
28+
id = "UC1"
29+
30+
def validate(self, commit):
31+
title = commit.message.title
32+
m = _CONVENTIONAL.match(title)
33+
if not m:
34+
return []
35+
36+
ctype = m.group("type").lower()
37+
scope = m.group("scope").lower()
38+
key = (ctype, scope)
39+
40+
if key not in _FORBIDDEN:
41+
return []
42+
43+
suggestion = _FORBIDDEN[key]
44+
return [
45+
RuleViolation(
46+
self.id,
47+
f'"{ctype}({scope})" pollutes release notes with non-user-facing changes. '
48+
f"Use \"{suggestion}: ...\" instead.",
49+
line_nr=1,
50+
)
51+
]

gitlint_rules_test.py

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
"""Tests for the ForbiddenTypeScope gitlint rule."""
2+
3+
import pytest
4+
5+
from gitlint_rules.forbidden_type_scope import ForbiddenTypeScope
6+
from gitlint.rules import RuleViolation
7+
8+
9+
class FakeCommit:
10+
"""Minimal stand-in for a gitlint commit object."""
11+
12+
def __init__(self, title):
13+
self.message = type("msg", (), {"title": title})()
14+
15+
16+
def run_rule(title):
17+
"""Run ForbiddenTypeScope against a commit title and return violations."""
18+
rule = ForbiddenTypeScope()
19+
commit = FakeCommit(title)
20+
return rule.validate(commit)
21+
22+
23+
# --- should be rejected ---
24+
25+
26+
@pytest.mark.parametrize(
27+
"title",
28+
[
29+
"fix(ci): update workflow",
30+
"feat(ci): add new job",
31+
"fix(e2e): repair test",
32+
"feat(e2e): add new test",
33+
],
34+
)
35+
def test_rejects_forbidden_combinations(title):
36+
violations = run_rule(title)
37+
assert violations, f"Expected violation for {title!r}"
38+
assert len(violations) == 1
39+
assert isinstance(violations[0], RuleViolation)
40+
41+
42+
def test_fix_ci_suggests_ci_subsystem():
43+
violations = run_rule("fix(ci): update workflow")
44+
assert "ci(<subsystem>)" in violations[0].message.lower()
45+
46+
47+
def test_feat_e2e_suggests_ci_e2e():
48+
violations = run_rule("feat(e2e): add new test")
49+
assert "ci(e2e)" in violations[0].message.lower()
50+
51+
52+
# --- should be allowed ---
53+
54+
55+
@pytest.mark.parametrize(
56+
"title",
57+
[
58+
"ci(lint): update linter config",
59+
"ci(e2e): fix flaky test",
60+
"fix(mint): correct token refresh",
61+
"feat(review-agent): add outcome labels",
62+
"chore(ci): bump action version",
63+
"test(e2e): add new scenario",
64+
"refactor(ci): simplify matrix",
65+
"docs: update readme",
66+
"fix(#123): handle nil pointer",
67+
],
68+
)
69+
def test_allows_valid_combinations(title):
70+
violations = run_rule(title)
71+
assert not violations, f"Unexpected violation for {title!r}: {violations}"
72+
73+
74+
# --- should not crash on non-conventional titles ---
75+
76+
77+
@pytest.mark.parametrize(
78+
"title",
79+
[
80+
"just a plain message",
81+
"WIP",
82+
"",
83+
],
84+
)
85+
def test_ignores_non_conventional(title):
86+
violations = run_rule(title)
87+
assert not violations

0 commit comments

Comments
 (0)