Skip to content

Commit 83e76f9

Browse files
committed
ci: check commit messages in pull requests
Unfortunately Github does not allow commenting on commit messages directly. At least perform basic checks to enforce our rules: * titles should be less than 72 characters * titles should start with a short lower case prefix to mention the "topic" of the commit. * no capitalization nor punctuation in the commit title * all commits should have English prose describing the changes. * all commits should be signed-off-by their author (git commit -s). * the list of sanctioned commit trailers is enforced. * referencing github issues/pull_requests must be done via full urls. * referenced commit ids must be valid. Signed-off-by: Robin Jarry <[email protected]>
1 parent f09ed11 commit 83e76f9

File tree

4 files changed

+205
-17
lines changed

4 files changed

+205
-17
lines changed

.github/workflows/ci.yml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,19 @@ jobs:
2222
- run: python -m pip install --upgrade tox
2323
- run: python -m tox -e lint
2424

25+
check-commits:
26+
if: github.event_name == 'pull_request'
27+
runs-on: ubuntu-latest
28+
env:
29+
LYPY_START_COMMIT: "${{ github.event.pull_request.base.sha }}"
30+
LYPY_END_COMMIT: "${{ github.event.pull_request.head.sha }}"
31+
steps:
32+
- run: sudo apt-get install git make
33+
- uses: actions/checkout@v4
34+
with:
35+
fetch-depth: 0
36+
- run: make check-commits
37+
2538
test:
2639
runs-on: ubuntu-20.04
2740
strategy:

Makefile

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,10 @@ tests:
1212
format:
1313
tox -e format
1414

15-
.PHONY: lint tests format
15+
LYPY_START_COMMIT ?= origin/master
16+
LYPY_END_COMMIT ?= HEAD
17+
18+
check-commits:
19+
./check-commits.sh $(LYPY_START_COMMIT)..$(LYPY_END_COMMIT)
20+
21+
.PHONY: lint tests format check-commits

README.rst

Lines changed: 55 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ Here are the steps for submitting a change in the code base:
232232

233233
#. Create a new branch named after what your are working on::
234234

235-
git checkout -b my-topic
235+
git checkout -b my-topic -t origin/master
236236

237237
#. Edit the code and call ``make format`` to ensure your modifications comply
238238
with the `coding style`__.
@@ -251,21 +251,60 @@ Here are the steps for submitting a change in the code base:
251251
your changes do not break anything. You can also run ``make`` which will run
252252
both.
253253

254-
#. Create commits by following these simple guidelines:
255-
256-
- Solve only one problem per commit.
257-
- Use a short (less than 72 characters) title on the first line followed by
258-
an blank line and a more thorough description body.
259-
- Wrap the body of the commit message should be wrapped at 72 characters too
260-
unless it breaks long URLs or code examples.
261-
- If the commit fixes a Github issue, include the following line::
262-
263-
Fixes: #NNNN
264-
265-
Inspirations:
266-
267-
https://chris.beams.io/posts/git-commit/
268-
https://wiki.openstack.org/wiki/GitCommitMessages
254+
#. Once you are happy with your work, you can create a commit (or several
255+
commits). Follow these general rules:
256+
257+
- Address only one issue/topic per commit.
258+
- Describe your changes in imperative mood, e.g. *"make xyzzy do frotz"*
259+
instead of *"[This patch] makes xyzzy do frotz"* or *"[I] changed xyzzy to
260+
do frotz"*, as if you are giving orders to the codebase to change its
261+
behaviour.
262+
- Limit the first line (title) of the commit message to 60 characters.
263+
- Use a short prefix for the commit title for readability with ``git log
264+
--oneline``. Do not use the `fix:` nor `feature:` prefixes. See recent
265+
commits for inspiration.
266+
- Only use lower case letters for the commit title except when quoting
267+
symbols or known acronyms.
268+
- Use the body of the commit message to actually explain what your patch
269+
does and why it is useful. Even if your patch is a one line fix, the
270+
description is not limited in length and may span over multiple
271+
paragraphs. Use proper English syntax, grammar and punctuation.
272+
- If you are fixing an issue, use appropriate ``Closes: <URL>`` or
273+
``Fixes: <URL>`` trailers.
274+
- If you are fixing a regression introduced by another commit, add a
275+
``Fixes: <COMMIT_ID> ("<TITLE>")`` trailer.
276+
- When in doubt, follow the format and layout of the recent existing
277+
commits.
278+
- The following trailers are accepted in commits. If you are using multiple
279+
trailers in a commit, it's preferred to also order them according to this
280+
list.
281+
282+
* ``Closes: <URL>``: close the referenced issue or pull request.
283+
* ``Fixes: <SHA> ("<TITLE>")``: reference the commit that introduced
284+
a regression.
285+
* ``Link: <URL>``: any useful link to provide context for your commit.
286+
* ``Suggested-by``
287+
* ``Requested-by``
288+
* ``Reported-by``
289+
* ``Co-authored-by``
290+
* ``Tested-by``
291+
* ``Reviewed-by``
292+
* ``Acked-by``
293+
* ``Signed-off-by``: Compulsory!
294+
295+
There is a great reference for commit messages in the `Linux kernel
296+
documentation`__.
297+
298+
__ https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
299+
300+
IMPORTANT: you must sign-off your work using ``git commit --signoff``. Follow
301+
the `Linux kernel developer's certificate of origin`__ for more details. All
302+
contributions are made under the MIT license. If you do not want to disclose
303+
your real name, you may sign-off using a pseudonym. Here is an example::
304+
305+
Signed-off-by: Robin Jarry <[email protected]>
306+
307+
__ https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
269308

270309
#. Push your topic branch in your forked repository::
271310

check-commits.sh

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
#!/bin/sh
2+
3+
set -e
4+
5+
revision_range="${1?revision range}"
6+
7+
valid=0
8+
revisions=$(git rev-list --reverse "$revision_range")
9+
total=$(echo $revisions | wc -w)
10+
if [ "$total" -eq 0 ]; then
11+
exit 0
12+
fi
13+
tmp=$(mktemp)
14+
trap "rm -f $tmp" EXIT
15+
16+
allowed_trailers="
17+
Closes
18+
Fixes
19+
Link
20+
Suggested-by
21+
Requested-by
22+
Reported-by
23+
Co-authored-by
24+
Signed-off-by
25+
Tested-by
26+
Reviewed-by
27+
Acked-by
28+
"
29+
30+
n=0
31+
title=
32+
shortrev=
33+
fail=false
34+
repo=CESNET/libyang-python
35+
repo_url=https://github.com/$repo
36+
api_url=https://api.github.com/repos/$repo
37+
38+
err() {
39+
40+
echo "error: commit $shortrev (\"$title\") $*" >&2
41+
fail=true
42+
}
43+
44+
check_issue() {
45+
json=$(curl -f -X GET -L --no-progress-meter \
46+
-H "Accept: application/vnd.github+json" \
47+
-H "X-GitHub-Api-Version: 2022-11-28" \
48+
"$api_url/issues/${1##*/}") || return 1
49+
test $(echo "$json" | jq -r .state) = open
50+
}
51+
52+
for rev in $revisions; do
53+
n=$((n + 1))
54+
title=$(git log --format='%s' -1 "$rev")
55+
fail=false
56+
shortrev=$(printf '%-12.12s' $rev)
57+
58+
if [ "$(echo "$title" | wc -m)" -gt 72 ]; then
59+
err "title is longer than 72 characters, please make it shorter"
60+
fi
61+
if ! echo "$title" | grep -qE '^[a-z0-9,{}/_-]+: '; then
62+
err "title lacks a lowercase topic prefix (e.g. 'data: ')"
63+
fi
64+
if echo "$title" | grep -qE '^[a-z0-9,{}/_-]+: [A-Z][a-z]'; then
65+
err "title starts with an capital letter, please use lower case"
66+
fi
67+
if ! echo "$title" | grep -qE '[A-Za-z0-9]$'; then
68+
err "title ends with punctuation, please remove it"
69+
fi
70+
71+
author=$(git log --format='%an <%ae>' -1 "$rev")
72+
if ! git log --format="%(trailers:key=Signed-off-by,only,valueonly,unfold)" -1 "$rev" |
73+
grep -qFx "$author"; then
74+
err "'Signed-off-by: $author' trailer is missing"
75+
fi
76+
77+
for trailer in $(git log --format="%(trailers:only,keyonly)" -1 "$rev"); do
78+
if ! echo "$allowed_trailers" | grep -qFx "$trailer"; then
79+
err "trailer '$trailer' is misspelled or not in the sanctioned list"
80+
fi
81+
done
82+
83+
git log --format="%(trailers:key=Closes,only,valueonly,unfold)" -1 "$rev" > $tmp
84+
while read -r value; do
85+
if [ -z "$value" ]; then
86+
continue
87+
fi
88+
case "$value" in
89+
$repo_url/*/[0-9]*)
90+
if ! check_issue "$value"; then
91+
err "'$value' does not reference a valid open issue"
92+
fi
93+
;;
94+
\#[0-9]*)
95+
err "please use the full issue URL: 'Closes: $repo_url/issues/$value'"
96+
;;
97+
*)
98+
err "invalid trailer value '$value'. The 'Closes:' trailer must only be used to reference issue URLs"
99+
;;
100+
esac
101+
done < "$tmp"
102+
103+
git log --format="%(trailers:key=Fixes,only,valueonly,unfold)" -1 "$rev" > $tmp
104+
while read -r value; do
105+
if [ -z "$value" ]; then
106+
continue
107+
fi
108+
fixes_rev=$(echo "$value" | sed -En 's/([A-Fa-f0-9]{7,}[[:space:]]\(".*"\))/\1/p')
109+
if ! git cat-file commit "$fixes_rev" >/dev/null; then
110+
err "trailer '$value' does not refer to a known commit"
111+
fi
112+
done < "$tmp"
113+
114+
body=$(git log --format='%b' -1 "$rev")
115+
body=${body%$(git log --format='%(trailers)' -1 "$rev")}
116+
if [ "$(echo "$body" | wc -w)" -lt 3 ]; then
117+
err "body has less than three words, please describe your changes"
118+
fi
119+
120+
if [ "$fail" = true ]; then
121+
continue
122+
fi
123+
echo "ok commit $shortrev (\"$title\")"
124+
valid=$((valid + 1))
125+
done
126+
127+
echo "$valid/$total valid commit messages"
128+
if [ "$valid" -ne "$total" ]; then
129+
exit 1
130+
fi

0 commit comments

Comments
 (0)