← Back to team overview

kicad-developers team mailing list archive

Re: A reminder on Git commit comments

 

This is why we have a commit message policy.  Obviously this isn't a
perfect solution.  Feel free to adopt this policy for library developer
commits.  There is always the hard line approach where you kick the
merge request back to the submitted and ask them to fix the commit
message or use `git commit --amend` to fix the commit message yourself.

On 5/1/2018 6:03 PM, Reece R. Pollack wrote:
> I'd like to make an observation on some of the Git commit comments I'm
> reading. Some of these commit log entries are really unhelpful.
> 
> When I was at University many, many years ago, students were taught to
> place comments in their code. Many misunderstood the purpose of
> comments, though, and would write code like this:
> 
>     x = x + 1;       /* Add 1 to x */
> 
> 
> While the comment is totally correct, it's also totally useless. I can
> /see/ that this line adds 1 to /x/; I don't need a comment to tell me
> that. As a programmer coming along later, what I need to know is /why/ 1
> is being added to /x/.
> 
> I'm seeing commit comments of the form "add one to x" in the KiCad
> repos. These comments are useless. For an example, look at this commit
> in the kicad-symbols repo:
> 
>     commit 4ada78e26774c841ce8ec33e8b221d04fed1f4c7
>     Author: Rene Pöschl <r.poeschl@xxxxxxxxxxxxxxxxx>
>     Date:   Fri Apr 20 11:14:47 2018 +0200
> 
>         Remove Connector_Specialized files.
> 
> Great. I can /see/ that these files are being removed. That's obvious
> from looking at the content of the commit. But there is no explanation
> of why these files were deleted. Don't tell me that there was discussion
> about this in email; even if that happened it's irrelevant. No one
> should have to go chasing through an email archive hoping to understand
> why a change was made. That's what the commit log entry is for.
> 
> I went back further in the Git logs for commits touching these files
> hoping to find a more illuminating commit comment, but found nothing
> helpful. In fact, I came across an earlier series of commits that are
> even /less/ helpful:
> 
>     commit 467170586b106d00c4df185dc1683046e259b74c
>     Author: evanshultz <evanshultz@xxxxxxxxxxxxxxxxxxxxxxxx>
>     Date:   Thu Apr 19 09:42:52 2018 -0700
> 
>         Just checking... still no dice
> 
>     commit 966a1a40aa36732d9c2404b044a4ef63bd0bb417
>     Author: evanshultz <evanshultz@xxxxxxxxxxxxxxxxxxxxxxxx>
>     Date:   Thu Apr 19 09:36:28 2018 -0700
> 
>         Delete Connector_Specialized.lib
> 
>     commit aeb4270213da8a063f24163d6fa31e8521f08a83
>     Author: evanshultz <evanshultz@xxxxxxxxxxxxxxxxxxxxxxxx>
>     Date:   Thu Apr 19 09:36:03 2018 -0700
> 
>         Delete Connector_Specialized.dcm
> 
> The top (latest) commit actually reverts the actions of the previous two
> commits. So now I have the idea that there's some sort of disagreement
> whether these files should be part of the repo or not. But I still have
> no idea why.
> 
> In a previous job I was the poor sod (or arrogant bastard, take your
> pick) who had to review commits before they were merged into the master
> branch. I would have rejected commits like these immediately. With Open
> Source development there's often less control over what gets merged and
> what doesn't. But that just means it's all the more important for
> everyone to make an effort to write good commit comments.
> 
> From the KiCad source file
> Documentation/development/commit-message-format.md:
> 
>     Commit messages should begin with a subject line; try to limit this
>     to no more
>     than 50-72 characters. The body of the message should be separated
>     from the
>     subject by a blank line and wrapped at 72 characters. The body of a
>     commit
>     message should explain what the commit does and why, but do not
>     explain *how*
>     as the code itself should do that.
> 
> 
> 
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
> 


Follow ups

References