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
  1. scorbajio commented at 9:20 pm on July 3, 2020: none
    Write linter to check that commit messages have a new line before the body or no body at all. fixes issue #19091.
  2. DrahtBot added the label Tests on Jul 3, 2020
  3. fanquake removed the label Tests on Jul 3, 2020
  4. fanquake added the label Scripts and tools on Jul 3, 2020
  5. scorbajio force-pushed on Jul 6, 2020
  6. 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.
  7. in test/lint/lint-git-commit-check.sh:2 in acc86bd57a outdated 
    0@@ -0,0 +1,27 @@
1+#!/usr/bin/env bash
2+# Copyright (c) 2017-2019 The Bitcoin Core developers
    adamjonas commented at 11:50 pm on July 6, 2020:
    I think you can just put 2020 here.
    scorbajio commented at 3:13 pm on July 8, 2020:
    fixed
  8. 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 to git 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
  9. 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
  10. adamjonas commented at 0:36 am on July 7, 2020: member
    Warm welcome @Ghorbanian and excited to see more contributions for you in the future!
  11. 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 > 706

    Two 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.
  12. 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
  13. 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
  14. 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.

  15. scorbajio force-pushed on Jul 8, 2020
  16. scorbajio commented at 3:19 pm on July 8, 2020: none
    @troygiorshev Thank you for the code review!
  17. 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 to TRAVIS_COMMIT_RANGE if available?
    scorbajio commented at 8:18 pm on July 12, 2020:
    fixed
  18. scorbajio force-pushed on Jul 10, 2020
  19. scorbajio force-pushed on Jul 12, 2020
  20. troygiorshev changes_requested
  21. troygiorshev commented at 1:25 pm on July 15, 2020: contributor
    Below \/
  22. 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."
  23. troygiorshev changes_requested
  24. troygiorshev commented at 1:25 pm on July 15, 2020: contributor

    Reviewed, manually tested. Works great!

    Just need the message changed.

  25. 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.
    284a969cc0
  26. scorbajio force-pushed on Jul 15, 2020
  27. troygiorshev commented at 1:53 pm on July 16, 2020: contributor
    ACK 284a969cc082ae3c63ab523f22e71da86ad4ab20 Reviewed, manually tested. Works great!
  28. 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.
  29. adamjonas commented at 2:43 pm on July 23, 2020: member
    utACK 284a969cc082ae3c63ab523f22e71da86ad4ab20
  30. MarcoFalke merged this on Jul 30, 2020
  31. MarcoFalke closed this on Jul 30, 2020
  32. sidhujag referenced this in commit 0cd09e57ad on Jul 31, 2020
  33. fjahr commented at 10:25 pm on August 2, 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?
  34. 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

  35. MarcoFalke referenced this in commit 65e4ecabd5 on Aug 5, 2020
  36. PastaPastaPasta referenced this in commit f915d88046 on Jun 27, 2021
  37. PastaPastaPasta referenced this in commit 4941ab2a83 on Jun 27, 2021
  38. PastaPastaPasta referenced this in commit a867a7fc74 on Jun 28, 2021
  39. PastaPastaPasta referenced this in commit a4e7a057d7 on Jun 28, 2021
  40. PastaPastaPasta referenced this in commit ec0bc05cc4 on Jun 29, 2021
  41. PastaPastaPasta referenced this in commit 02ece69a88 on Jun 29, 2021
  42. PastaPastaPasta referenced this in commit 110cc8e9fe on Jul 1, 2021
  43. PastaPastaPasta referenced this in commit 006cd52e74 on Jul 1, 2021
  44. PastaPastaPasta referenced this in commit fd4513868b on Jul 1, 2021
  45. PastaPastaPasta referenced this in commit eb6893fa01 on Jul 1, 2021
  46. PastaPastaPasta referenced this in commit 4e07a366c2 on Jul 15, 2021
  47. PastaPastaPasta referenced this in commit 731fc7a695 on Jul 15, 2021
  48. PastaPastaPasta referenced this in commit 7a92af887d on Jul 16, 2021
  49. PastaPastaPasta referenced this in commit fb7b4bbdca on Jul 16, 2021
  50. DrahtBot locked this on Aug 16, 2022


scorbajio adamjonas troygiorshev MarcoFalke fjahr

Labels
Scripts and tools

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-10-05 01:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me