← Back to team overview

kicad-developers team mailing list archive

A reminder on Git commit comments

 

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.



Follow ups