Automatically checking commit messages

Automatically checking commit messages

[Arun Isaac]

I have been working on a guile script to automatically check commit
messages -- something like `guix lint' but for commit messages instead
of package definitions. This could help us enforce our commit message
guidelines and avoid screw-ups like the one I did in commit
1ee879e96705e6381c056358b7f426f2cf99c1df. I believe more automation is
essential and would help us scale better if/when we have more people
with commit access.

I have taken the following approach with my script: I have devised a
grammar (shown below) to parse commit messages. Once the parser outputs
a parse tree for the commit message, we can apply any number of checks
on it.

Do you think this is a good approach? If so, I shall proceed with the
work, and complete the script. If not, what other approach would be
good?

Note that the grammar shown below is incomplete and buggy. Do ignore
that for now.

(define-peg-string-patterns
  "commit <-- module S module S summary NL NL (description NL NL)? changelog signature?
module <-- (!C !S .)* C
summary <-- (!NL .)*
description <-- (!(NL NL) .)*
changelog <-- entry*
entry <-- bullet S file S section C S change
file <-- word
section <-- LR (!RR .)* RR
change <-- (!(bullet / (NL NL)) .)*
signature <-- signedoffby signatory S email
signedoffby < 'Signed-off-by: '
signatory <-- (!' <' .)*
email <-- LA (!RA .)* RA
word <- (!S .)*
S < ' '
C < ':'
NL < '\n'
bullet < '*'
LR < '('
RR < ')'
LA < '<'
RA < '>'")

Re: Automatically checking commit messages

[Jelle Licht]

[-- Attachment #1: Type: text/plain, Size: 2008 bytes --]

2017-09-20 17:47 GMT+02:00 Arun Isaac <arunisaac@systemreboot.net>:

>
> I have been working on a guile script to automatically check commit
> messages -- something like `guix lint' but for commit messages instead
> of package definitions. This could help us enforce our commit message
> guidelines and avoid screw-ups like the one I did in commit
> 1ee879e96705e6381c056358b7f426f2cf99c1df. I believe more automation is
> essential and would help us scale better if/when we have more people
> with commit access.
>
Hear, hear.

>
> I have taken the following approach with my script: I have devised a
> grammar (shown below) to parse commit messages. Once the parser outputs
> a parse tree for the commit message, we can apply any number of checks
> on it.
>
> Do you think this is a good approach? If so, I shall proceed with the
> work, and complete the script. If not, what other approach would be
> good?
>

Nice. What parts of the commit message guidelines do you expect to be
verifiable, and which parts do you think will be too hard/restrictive to
automatically verify?

>
> Note that the grammar shown below is incomplete and buggy. Do ignore
> that for now.
>
Alright. If you haven't done so already, adding test cases (from
existing proper and improper commit messages) would ease understanding.

>
> (define-peg-string-patterns
>   "commit <-- module S module S summary NL NL (description NL NL)?
> changelog signature?
> module <-- (!C !S .)* C
> summary <-- (!NL .)*
> description <-- (!(NL NL) .)*
> changelog <-- entry*
> entry <-- bullet S file S section C S change
> file <-- word
> section <-- LR (!RR .)* RR
> change <-- (!(bullet / (NL NL)) .)*
> signature <-- signedoffby signatory S email
> signedoffby < 'Signed-off-by: '
> signatory <-- (!' <' .)*
> email <-- LA (!RA .)* RA
> word <- (!S .)*
> S < ' '
> C < ':'
> NL < '\n'
> bullet < '*'
> LR < '('
> RR < ')'
> LA < '<'
> RA < '>'")


If this works, I would love for it to be a commit hook. Thanks for looking
into this!

- Jelle

[-- Attachment #2: Type: text/html, Size: 3215 bytes --]

Re: Automatically checking commit messages

[Vincent Legoll]

Hello

On Wed, Sep 20, 2017 at 5:47 PM, Arun Isaac <arunisaac@systemreboot.net> wrote:
> I have been working on a guile script to automatically check commit
> messages -- something like `guix lint' but for commit messages instead
> of package definitions. This could help us enforce our commit message
> guidelines and avoid screw-ups like the one I did in commit
> 1ee879e96705e6381c056358b7f426f2cf99c1df. I believe more automation is
> essential and would help us scale better if/when we have more people
> with commit access.

This is a good idea, and it would be even better if that same thing
could be used to generate a commit message with all the automatable
pieces pre-filled, with only the human parts left to the user.

-- 
Vincent Legoll

Re: Automatically checking commit messages

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 924 bytes --]

Vincent Legoll <vincent.legoll@gmail.com> writes:

> Hello
>
> On Wed, Sep 20, 2017 at 5:47 PM, Arun Isaac <arunisaac@systemreboot.net> wrote:
>> I have been working on a guile script to automatically check commit
>> messages -- something like `guix lint' but for commit messages instead
>> of package definitions. This could help us enforce our commit message
>> guidelines and avoid screw-ups like the one I did in commit
>> 1ee879e96705e6381c056358b7f426f2cf99c1df. I believe more automation is
>> essential and would help us scale better if/when we have more people
>> with commit access.
>
> This is a good idea, and it would be even better if that same thing
> could be used to generate a commit message with all the automatable
> pieces pre-filled, with only the human parts left to the user.

FWIW 'magit' can already fill out file names and variables by navigating
to the hunks in the diff buffer and pressing 'C'.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: Automatically checking commit messages

[Arun Isaac]

> What parts of the commit message guidelines do you expect to be
> verifiable, and which parts do you think will be too hard/restrictive
> to automatically verify?

I haven't thought too hard about this. I thought that, if we have a good
parser for commit messages, we can later add any number of
tests. Anyways, here are a few tests that immediately come to mind:

* Check for commit signature. Our existing pre-push hook already does
  this.
* Verify that the modules listed on the first line of the commit message
  ("gnu: you-get", "guix: build:", etc.) are accurate.
* Verify mundane things like capitalization of first word of sentence,
  period at the end of a sentence, no indentation when a changelog entry
  continues for more than one line, etc.
* Maybe, enforce templates for common commits like updating a package,
  enabling/disabling tests/parallel-build/parallel-tests, etc.
* Verify that the changelog lists all changes in the commit. This might
  require actually parsing the patch, and may not be easy.

> If you haven't done so already, adding test cases (from existing
> proper and improper commit messages) would ease understanding.

I will work on this script further, and post a patch once I have
something more complete.

>> This is a good idea, and it would be even better if that same thing
>> could be used to generate a commit message with all the automatable
>> pieces pre-filled, with only the human parts left to the user.

This is an interesting thought. I also just found out that git supports
commit message templates, and I wonder if it could be useful.

https://robots.thoughtbot.com/better-commit-messages-with-a-gitmessage-template

> FWIW 'magit' can already fill out file names and variables by navigating
> to the hunks in the diff buffer and pressing 'C'.

True, magit is very useful in that regard. But, at least for common
commits like updating patches, it would help to have something more
automated than that.