script: Linter to check commit message formatting #19439
pull scorbajio wants to merge 1 commits into bitcoin:master from scorbajio:linter-addition changing 1 files +46 −0-
scorbajio commented at 9:20 pm on July 3, 2020: noneWrite linter to check that commit messages have a new line before the body or no body at all. fixes issue #19091.
-
DrahtBot added the label Tests on Jul 3, 2020
-
fanquake removed the label Tests on Jul 3, 2020
-
fanquake added the label Scripts and tools on Jul 3, 2020
-
scorbajio force-pushed on Jul 6, 2020
-
in test/lint/lint-git-commit-check.sh:11 in acc86bd57a outdated
6+# Linter to check that commit messages have a new line before the body 7+# or no body at all 8+# 9+# reference: gist.github.com/agnivade/67b42d664ece2d4210c7 10+# 11+# WARNING: Using a "+" character in commit messages can result in
adamjonas commented at 11:45 pm on July 6, 2020:This line is failing the linter for having a trailing whitespace.
scorbajio commented at 3:13 pm on July 8, 2020:Fixed. -
in test/lint/lint-git-commit-check.sh:25 in acc86bd57a outdated
20+ if [ $n_line -eq 2 ] && [ $length -ne 0 ]; then 21+ echo "Your subject line follows a non-empty line. Subject lines should always be followed by a blank line." 22+ exit 1 23+ fi 24+ done < <(echo "$msg") 25+done < <(git log origin..HEAD --format=%s%b+)
adamjonas commented at 0:25 am on July 7, 2020:$ git log origin..HEAD --format=%s%b+
isn’t working for me while testing locally. I get:0fatal: ambiguous argument 'origin..HEAD': unknown revision or path not in the working tree. 1Use '--' to separate paths from revisions, like this: 2'git <command> [<revision>...] -- [<file>...]'
troygiorshev commented at 4:53 pm on July 7, 2020:Check out what this outputs, I don’t think it’s what you want. Possibly you’re looking for--format=%B+
, and then see if you can think of a way to deal with the “+” issue.
scorbajio commented at 3:16 pm on July 8, 2020:I changed it togit log origin/master..HEAD
. My intent is to get the last N commits that have not yet been pushed in a portable way. I did some research and I believe the change that I have made achieves this, but please let me know if not.
scorbajio commented at 3:17 pm on July 8, 2020:fixed -
in test/lint/lint-git-commit-check.sh:27 in acc86bd57a outdated
22+ exit 1 23+ fi 24+ done < <(echo "$msg") 25+done < <(git log origin..HEAD --format=%s%b+) 26+ 27+echo "No commit message formatting errors detected."
adamjonas commented at 0:25 am on July 7, 2020:If successfully passed, you’d want this to exit silently. Following the format of the other linters, I’d suggest replacing this line:
0exit ${EXIT_CODE}
scorbajio commented at 3:16 pm on July 8, 2020:fixed -
adamjonas commented at 0:36 am on July 7, 2020: memberWarm welcome @Ghorbanian and excited to see more contributions for you in the future!
-
in test/lint/lint-git-commit-check.sh:15 in acc86bd57a outdated
10+# 11+# WARNING: Using a "+" character in commit messages can result in 12+# improper parsing 13+ 14+export LC_ALL=C 15+while read -d "+" -r msg || [[ -n "$msg" ]]; do
troygiorshev commented at 4:51 pm on July 7, 2020:This “+” idea is clever! However, I don’t think it’s one we can live with. The “+” character is actually used surprisingly often in commit messages.
git log --grep='+' --oneline | wc -l
> 706Two suggested solutions, if you’re looking for somewhere to start
- Try and find a list of valid characters in commit messages, and see if you can use one that isn’t in the list. Maybe “#” would work? I know that emojis work in commit messages, so maybe this isn’t a viable option, but check it out :)
- Try iterate over each commit, as opposed to getting them all at once.
scorbajio commented at 3:17 pm on July 8, 2020:After looking at the script some more, I decided to iterate over each commit by hash value in order to circumvent this delimiter problem. Thank you for the suggestion. -
in test/lint/lint-git-commit-check.sh:21 in acc86bd57a outdated
16+ n_line=0 17+ while IFS= read -r line || [[ -n "$line" ]]; do 18+ n_line=$((n_line+1)) 19+ length=${#line} 20+ if [ $n_line -eq 2 ] && [ $length -ne 0 ]; then 21+ echo "Your subject line follows a non-empty line. Subject lines should always be followed by a blank line."
troygiorshev commented at 4:55 pm on July 7, 2020:This message isn’t quite right. Maybe something like one of these is better.
0 echo "Your body follows a non-empty line. Subject lines should always be followed by a blank line."
0 echo "Your subject line is followed by a non-empty line. Subject lines should always be followed by a blank line."
scorbajio commented at 3:17 pm on July 8, 2020:fixed -
in test/lint/lint-git-commit-check.sh:9 in acc86bd57a outdated
0@@ -0,0 +1,27 @@ 1+#!/usr/bin/env bash 2+# Copyright (c) 2017-2019 The Bitcoin Core developers 3+# Distributed under the MIT software license, see the accompanying 4+# file COPYING or http://www.opensource.org/licenses/mit-license.php. 5+# 6+# Linter to check that commit messages have a new line before the body 7+# or no body at all 8+# 9+# reference: gist.github.com/agnivade/67b42d664ece2d4210c7
troygiorshev commented at 4:55 pm on July 7, 2020:Thanks for including this reference! Let’s keep it out of the code though, can you move it to the body of your commit message?
scorbajio commented at 3:18 pm on July 8, 2020:fixed -
troygiorshev commented at 4:56 pm on July 7, 2020: contributor
Hi @Ghorbanian thanks for picking this up!
When manually testing this script, I don’t think it actually works :/
I’ve suggested below why I think this may be happening.
-
scorbajio force-pushed on Jul 8, 2020
-
scorbajio commented at 3:19 pm on July 8, 2020: none@troygiorshev Thank you for the code review!
-
in test/lint/lint-git-commit-check.sh:24 in 6e8feaa3af outdated
19+ echo "Your body is followed by a non-empty line. Subject lines should always be followed by a blank line." 20+ EXIT_CODE=1 21+ break 22+ fi 23+ done < <(git log --format=%B -n 1 "$commit_hash") 24+done < <(git log origin/master..HEAD --format=%H)
MarcoFalke commented at 3:26 pm on July 8, 2020:Could make sense to default this toTRAVIS_COMMIT_RANGE
if available?
scorbajio commented at 8:18 pm on July 12, 2020:fixed -
scorbajio force-pushed on Jul 10, 2020
-
scorbajio force-pushed on Jul 12, 2020
-
troygiorshev changes_requested
-
troygiorshev commented at 1:25 pm on July 15, 2020: contributorBelow \/
-
in test/lint/lint-git-commit-check.sh:40 in a62b825369 outdated
35+ n_line=0 36+ while IFS= read -r line || [[ -n "$line" ]]; do 37+ n_line=$((n_line+1)) 38+ length=${#line} 39+ if [ $n_line -eq 2 ] && [ $length -ne 0 ]; then 40+ echo "The body of commit hash ${commit_hash} is followed by a non-empty line. Subject lines should always be followed by a blank line."
troygiorshev commented at 1:25 pm on July 15, 2020:Good idea adding the commit hash! The grammar still isn’t quite what I’d like.
0 echo "The subject line of commit hash ${commit_hash} is followed by a non-empty line. Subject lines should always be followed by a blank line."
or, if you like it better
0 echo "The subject line of commit hash ${commit_hash} is followed by a non-empty line. Please insert a blank line between the subject line and the body of the commit message."
-
troygiorshev changes_requested
-
troygiorshev commented at 1:25 pm on July 15, 2020: contributor
Reviewed, manually tested. Works great!
Just need the message changed.
-
Linter to check commit message formatting
Write linter to check that commit messages have a new line before the body or no body at all. reference: gist.github.com/agnivade/67b42d664ece2d4210c7 Fixes issue #19091.
-
scorbajio force-pushed on Jul 15, 2020
-
troygiorshev commented at 1:53 pm on July 16, 2020: contributorACK 284a969cc082ae3c63ab523f22e71da86ad4ab20 Reviewed, manually tested. Works great!
-
fjahr commented at 12:46 pm on July 17, 2020: member
tested ACK 284a969cc082ae3c63ab523f22e71da86ad4ab20
0$ touch foo 1$ git add foo 2$ git commit -am "Foo 3→ Bar" 4$ test/lint/lint-git-commit-check.sh 5The subject line of commit hash 1b6e02448bcced186160815a986d65ca38144940 is followed by a non-empty line. Subject lines should always be followed by a blank line. 6$ test/lint/lint-git-commit-check.sh 0 7$ test/lint/lint-git-commit-check.sh 1 8The subject line of commit hash 1b6e02448bcced186160815a986d65ca38144940 is followed by a non-empty line. Subject lines should always be followed by a blank line.
-
adamjonas commented at 2:43 pm on July 23, 2020: memberutACK 284a969cc082ae3c63ab523f22e71da86ad4ab20
-
MarcoFalke merged this on Jul 30, 2020
-
MarcoFalke closed this on Jul 30, 2020
-
sidhujag referenced this in commit 0cd09e57ad on Jul 31, 2020
-
fjahr commented at 10:25 pm on August 2, 2020: memberThe linter seems to fail in the CI for commit messages without a body, at least it did for me here https://travis-ci.org/github/bitcoin/bitcoin/builds/714296380 and I can not reproduce it locally. Can somebody else confirm this or is able to reproduce this locally?
-
fjahr commented at 8:57 am on August 4, 2020: member
The linter seems to fail in the CI for commit messages without a body, at least it did for me here https://travis-ci.org/github/bitcoin/bitcoin/builds/714296380 and I can not reproduce it locally. Can somebody else confirm this or is able to reproduce this locally?
My initial analysis of what caused the failure was wrong so ignore that part. But I think I have found what the issue was and proposed a fix here: https://github.com/bitcoin/bitcoin/pull/19654
-
MarcoFalke referenced this in commit 65e4ecabd5 on Aug 5, 2020
-
PastaPastaPasta referenced this in commit f915d88046 on Jun 27, 2021
-
PastaPastaPasta referenced this in commit 4941ab2a83 on Jun 27, 2021
-
PastaPastaPasta referenced this in commit a867a7fc74 on Jun 28, 2021
-
PastaPastaPasta referenced this in commit a4e7a057d7 on Jun 28, 2021
-
PastaPastaPasta referenced this in commit ec0bc05cc4 on Jun 29, 2021
-
PastaPastaPasta referenced this in commit 02ece69a88 on Jun 29, 2021
-
PastaPastaPasta referenced this in commit 110cc8e9fe on Jul 1, 2021
-
PastaPastaPasta referenced this in commit 006cd52e74 on Jul 1, 2021
-
PastaPastaPasta referenced this in commit fd4513868b on Jul 1, 2021
-
PastaPastaPasta referenced this in commit eb6893fa01 on Jul 1, 2021
-
PastaPastaPasta referenced this in commit 4e07a366c2 on Jul 15, 2021
-
PastaPastaPasta referenced this in commit 731fc7a695 on Jul 15, 2021
-
PastaPastaPasta referenced this in commit 7a92af887d on Jul 16, 2021
-
PastaPastaPasta referenced this in commit fb7b4bbdca on Jul 16, 2021
-
DrahtBot locked this on Aug 16, 2022