From 46f6087a05a11707618a4101844af0c6af0f98c7 Mon Sep 17 00:00:00 2001 From: Robin Jarry Date: Thu, 28 Mar 2024 00:40:55 +0100 Subject: [PATCH 1/2] 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 --- .github/workflows/ci.yml | 13 ++++ CONTRIBUTING.rst | 71 ++++++++++++++++----- Makefile | 8 ++- check-commits.sh | 130 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 205 insertions(+), 17 deletions(-) create mode 100755 check-commits.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b4b8a29..6d84082 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,6 +22,19 @@ jobs: - run: python -m pip install --upgrade tox - run: python -m tox -e lint + check-commits: + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + env: + SRPY_START_COMMIT: "${{ github.event.pull_request.base.sha }}" + SRPY_END_COMMIT: "${{ github.event.pull_request.head.sha }}" + steps: + - run: sudo apt-get install git make + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - run: make check-commits + test: runs-on: ubuntu-20.04 strategy: diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 714598c..d9485ff 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -24,7 +24,7 @@ Here are the steps for submitting a change in the code base: #. Create a new branch named after what your are working on:: - git checkout -b my-topic + git checkout -b my-topic -t origin/master #. Edit the code and call ``make format`` to ensure your modifications comply with the `coding style`__. @@ -43,21 +43,60 @@ Here are the steps for submitting a change in the code base: your changes do not break anything. You can also run ``make`` which will run both. -#. Create commits by following these simple guidelines: - - - Solve only one problem per commit. - - Use a short (less than 72 characters) title on the first line followed by - an blank line and a more thorough description body. - - Wrap the body of the commit message should be wrapped at 72 characters too - unless it breaks long URLs or code examples. - - If the commit fixes a Github issue, include the following line:: - - Fixes: #NNNN - - Inspirations: - - https://chris.beams.io/posts/git-commit/ - https://wiki.openstack.org/wiki/GitCommitMessages +#. Once you are happy with your work, you can create a commit (or several + commits). Follow these general rules: + + - Address only one issue/topic per commit. + - Describe your changes in imperative mood, e.g. *"make xyzzy do frotz"* + instead of *"[This patch] makes xyzzy do frotz"* or *"[I] changed xyzzy to + do frotz"*, as if you are giving orders to the codebase to change its + behaviour. + - Limit the first line (title) of the commit message to 60 characters. + - Use a short prefix for the commit title for readability with ``git log + --oneline``. Do not use the `fix:` nor `feature:` prefixes. See recent + commits for inspiration. + - Only use lower case letters for the commit title except when quoting + symbols or known acronyms. + - Use the body of the commit message to actually explain what your patch + does and why it is useful. Even if your patch is a one line fix, the + description is not limited in length and may span over multiple + paragraphs. Use proper English syntax, grammar and punctuation. + - If you are fixing an issue, use appropriate ``Closes: `` or + ``Fixes: `` trailers. + - If you are fixing a regression introduced by another commit, add a + ``Fixes: ("")`` trailer. + - When in doubt, follow the format and layout of the recent existing + commits. + - The following trailers are accepted in commits. If you are using multiple + trailers in a commit, it's preferred to also order them according to this + list. + + * ``Closes: <URL>``: close the referenced issue or pull request. + * ``Fixes: <SHA> ("<TITLE>")``: reference the commit that introduced + a regression. + * ``Link: <URL>``: any useful link to provide context for your commit. + * ``Suggested-by`` + * ``Requested-by`` + * ``Reported-by`` + * ``Co-authored-by`` + * ``Tested-by`` + * ``Reviewed-by`` + * ``Acked-by`` + * ``Signed-off-by``: Compulsory! + + There is a great reference for commit messages in the `Linux kernel + documentation`__. + + __ https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes + + IMPORTANT: you must sign-off your work using ``git commit --signoff``. Follow + the `Linux kernel developer's certificate of origin`__ for more details. All + contributions are made under the MIT license. If you do not want to disclose + your real name, you may sign-off using a pseudonym. Here is an example:: + + Signed-off-by: Robin Jarry <robin@jarry.cc> + + __ https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin #. Push your topic branch in your forked repository:: diff --git a/Makefile b/Makefile index e20e29e..9722c0e 100644 --- a/Makefile +++ b/Makefile @@ -12,4 +12,10 @@ tests: format: tox -e format -.PHONY: lint tests format +SRPY_START_COMMIT ?= origin/master +SRPY_END_COMMIT ?= HEAD + +check-commits: + ./check-commits.sh $(SRPY_START_COMMIT)..$(SRPY_END_COMMIT) + +.PHONY: lint tests format check-commits diff --git a/check-commits.sh b/check-commits.sh new file mode 100755 index 0000000..39cc8d3 --- /dev/null +++ b/check-commits.sh @@ -0,0 +1,130 @@ +#!/bin/sh + +set -e + +revision_range="${1?revision range}" + +valid=0 +revisions=$(git rev-list --reverse "$revision_range") +total=$(echo $revisions | wc -w) +if [ "$total" -eq 0 ]; then + exit 0 +fi +tmp=$(mktemp) +trap "rm -f $tmp" EXIT + +allowed_trailers=" +Closes +Fixes +Link +Suggested-by +Requested-by +Reported-by +Co-authored-by +Signed-off-by +Tested-by +Reviewed-by +Acked-by +" + +n=0 +title= +shortrev= +fail=false +repo=sysrepo/sysrepo-python +repo_url=https://github.com/$repo +api_url=https://api.github.com/repos/$repo + +err() { + + echo "error: commit $shortrev (\"$title\") $*" >&2 + fail=true +} + +check_issue() { + json=$(curl -f -X GET -L --no-progress-meter \ + -H "Accept: application/vnd.github+json" \ + -H "X-GitHub-Api-Version: 2022-11-28" \ + "$api_url/issues/${1##*/}") || return 1 + test $(echo "$json" | jq -r .state) = open +} + +for rev in $revisions; do + n=$((n + 1)) + title=$(git log --format='%s' -1 "$rev") + fail=false + shortrev=$(printf '%-12.12s' $rev) + + if [ "$(echo "$title" | wc -m)" -gt 72 ]; then + err "title is longer than 72 characters, please make it shorter" + fi + if ! echo "$title" | grep -qE '^[a-z0-9,{}/_-]+: '; then + err "title lacks a lowercase topic prefix (e.g. 'data: ')" + fi + if echo "$title" | grep -qE '^[a-z0-9,{}/_-]+: [A-Z][a-z]'; then + err "title starts with an capital letter, please use lower case" + fi + if ! echo "$title" | grep -qE '[A-Za-z0-9]$'; then + err "title ends with punctuation, please remove it" + fi + + author=$(git log --format='%an <%ae>' -1 "$rev") + if ! git log --format="%(trailers:key=Signed-off-by,only,valueonly,unfold)" -1 "$rev" | + grep -qFx "$author"; then + err "'Signed-off-by: $author' trailer is missing" + fi + + for trailer in $(git log --format="%(trailers:only,keyonly)" -1 "$rev"); do + if ! echo "$allowed_trailers" | grep -qFx "$trailer"; then + err "trailer '$trailer' is misspelled or not in the sanctioned list" + fi + done + + git log --format="%(trailers:key=Closes,only,valueonly,unfold)" -1 "$rev" > $tmp + while read -r value; do + if [ -z "$value" ]; then + continue + fi + case "$value" in + $repo_url/*/[0-9]*) + if ! check_issue "$value"; then + err "'$value' does not reference a valid open issue" + fi + ;; + \#[0-9]*) + err "please use the full issue URL: 'Closes: $repo_url/issues/$value'" + ;; + *) + err "invalid trailer value '$value'. The 'Closes:' trailer must only be used to reference issue URLs" + ;; + esac + done < "$tmp" + + git log --format="%(trailers:key=Fixes,only,valueonly,unfold)" -1 "$rev" > $tmp + while read -r value; do + if [ -z "$value" ]; then + continue + fi + fixes_rev=$(echo "$value" | sed -En 's/([A-Fa-f0-9]{7,}[[:space:]]\(".*"\))/\1/p') + if ! git cat-file commit "$fixes_rev" >/dev/null; then + err "trailer '$value' does not refer to a known commit" + fi + done < "$tmp" + + body=$(git log --format='%b' -1 "$rev") + body=${body%$(git log --format='%(trailers)' -1 "$rev")} + if [ "$(echo "$body" | wc -w)" -lt 3 ]; then + err "body has less than three words, please describe your changes" + fi + + if [ "$fail" = true ]; then + continue + fi + echo "ok commit $shortrev (\"$title\")" + valid=$((valid + 1)) +done + +echo "$valid/$total valid commit messages" +if [ "$valid" -ne "$total" ]; then + exit 1 +fi From d1925e63706c870729623a295ac4c4c86162ef8f Mon Sep 17 00:00:00 2001 From: Robin Jarry <robin@jarry.cc> Date: Thu, 28 Mar 2024 00:41:28 +0100 Subject: [PATCH 2/2] editorconfig: fix bad syntax The proper indent_style value is "tab" not "tabs". Link: http://docs.editorconfig.org/en/master/editorconfig-format.html#properties Signed-off-by: Robin Jarry <robin@jarry.cc> --- .editorconfig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.editorconfig b/.editorconfig index b534393..37f96c4 100644 --- a/.editorconfig +++ b/.editorconfig @@ -21,13 +21,13 @@ indent_style = space indent_size = 3 [Makefile] -indent_style = tabs +indent_style = tab indent_size = tab [*.sh] -indent_style = tabs +indent_style = tab indent_size = tab [*.{h,c}] -indent_style = tabs +indent_style = tab indent_size = tab