Skip to content

Commit 7f4c477

Browse files
rjarryrjarry
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 ddac638 commit 7f4c477

File tree

4 files changed

+205
-17
lines changed

4 files changed

+205
-17
lines changed

‎.github/workflows/ci.yml

+13
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+
SRPY_START_COMMIT: "${{ github.event.pull_request.base.sha }}"
30+
SRPY_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:

‎CONTRIBUTING.rst

+55-16
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ Here are the steps for submitting a change in the code base:
2424

2525
#. Create a new branch named after what your are working on::
2626

27-
git checkout -b my-topic
27+
git checkout -b my-topic -t origin/master
2828

2929
#. Edit the code and call ``make format`` to ensure your modifications comply
3030
with the `coding style`__.
@@ -43,21 +43,60 @@ Here are the steps for submitting a change in the code base:
4343
your changes do not break anything. You can also run ``make`` which will run
4444
both.
4545

46-
#. Create commits by following these simple guidelines:
47-
48-
- Solve only one problem per commit.
49-
- Use a short (less than 72 characters) title on the first line followed by
50-
an blank line and a more thorough description body.
51-
- Wrap the body of the commit message should be wrapped at 72 characters too
52-
unless it breaks long URLs or code examples.
53-
- If the commit fixes a Github issue, include the following line::
54-
55-
Fixes: #NNNN
56-
57-
Inspirations:
58-
59-
https://chris.beams.io/posts/git-commit/
60-
https://wiki.openstack.org/wiki/GitCommitMessages
46+
#. Once you are happy with your work, you can create a commit (or several
47+
commits). Follow these general rules:
48+
49+
- Address only one issue/topic per commit.
50+
- Describe your changes in imperative mood, e.g. *"make xyzzy do frotz"*
51+
instead of *"[This patch] makes xyzzy do frotz"* or *"[I] changed xyzzy to
52+
do frotz"*, as if you are giving orders to the codebase to change its
53+
behaviour.
54+
- Limit the first line (title) of the commit message to 60 characters.
55+
- Use a short prefix for the commit title for readability with ``git log
56+
--oneline``. Do not use the `fix:` nor `feature:` prefixes. See recent
57+
commits for inspiration.
58+
- Only use lower case letters for the commit title except when quoting
59+
symbols or known acronyms.
60+
- Use the body of the commit message to actually explain what your patch
61+
does and why it is useful. Even if your patch is a one line fix, the
62+
description is not limited in length and may span over multiple
63+
paragraphs. Use proper English syntax, grammar and punctuation.
64+
- If you are fixing an issue, use appropriate ``Closes: <URL>`` or
65+
``Fixes: <URL>`` trailers.
66+
- If you are fixing a regression introduced by another commit, add a
67+
``Fixes: <COMMIT_ID> ("<TITLE>")`` trailer.
68+
- When in doubt, follow the format and layout of the recent existing
69+
commits.
70+
- The following trailers are accepted in commits. If you are using multiple
71+
trailers in a commit, it's preferred to also order them according to this
72+
list.
73+
74+
* ``Closes: <URL>``: close the referenced issue or pull request.
75+
* ``Fixes: <SHA> ("<TITLE>")``: reference the commit that introduced
76+
a regression.
77+
* ``Link: <URL>``: any useful link to provide context for your commit.
78+
* ``Suggested-by``
79+
* ``Requested-by``
80+
* ``Reported-by``
81+
* ``Co-authored-by``
82+
* ``Tested-by``
83+
* ``Reviewed-by``
84+
* ``Acked-by``
85+
* ``Signed-off-by``: Compulsory!
86+
87+
There is a great reference for commit messages in the `Linux kernel
88+
documentation`__.
89+
90+
__ https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
91+
92+
IMPORTANT: you must sign-off your work using ``git commit --signoff``. Follow
93+
the `Linux kernel developer's certificate of origin`__ for more details. All
94+
contributions are made under the MIT license. If you do not want to disclose
95+
your real name, you may sign-off using a pseudonym. Here is an example::
96+
97+
Signed-off-by: Robin Jarry <[email protected]>
98+
99+
__ https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
61100

62101
#. Push your topic branch in your forked repository::
63102

‎Makefile

+7-1
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+
SRPY_START_COMMIT ?= origin/master
16+
SRPY_END_COMMIT ?= HEAD
17+
18+
check-commits:
19+
./check-commits.sh $(SRPY_START_COMMIT)..$(SRPY_END_COMMIT)
20+
21+
.PHONY: lint tests format check-commits

‎check-commits.sh

+130
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=sysrepo/sysrepo-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)