← Back to team overview

kicad-developers team mailing list archive

Re: A reminder on Git commit comments

 

I believe most of the contributors to kicad-symbols actually do not read
this mailing list. I know Rene does, but this may be better addressed by
watching PRs on the kicad-symbols GitHub repository and helping
contributors better phrase their commit messages.

Adam Wolf

On Tue, May 1, 2018, 5:04 PM Reece R. Pollack <reece@xxxxxxx> 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>
> <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>
> <evanshultz@xxxxxxxxxxxxxxxxxxxxxxxx>
> Date:   Thu Apr 19 09:42:52 2018 -0700
>
>     Just checking... still no dice
>
> commit 966a1a40aa36732d9c2404b044a4ef63bd0bb417
> Author: evanshultz <evanshultz@xxxxxxxxxxxxxxxxxxxxxxxx>
> <evanshultz@xxxxxxxxxxxxxxxxxxxxxxxx>
> Date:   Thu Apr 19 09:36:28 2018 -0700
>
>     Delete Connector_Specialized.lib
>
> commit aeb4270213da8a063f24163d6fa31e8521f08a83
> Author: evanshultz <evanshultz@xxxxxxxxxxxxxxxxxxxxxxxx>
> <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
>

References