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
_______________________________________________
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
