On Thu, 22 Oct 2015 08:58:53 +0200 Paul Kocialkowski contact@paulk.fr wrote:
Hi Carl-Daniel,
Le dimanche 18 octobre 2015 à 20:58 +0200, Carl-Daniel Hailfinger a écrit :
On 18.10.2015 19:37, Paul Kocialkowski wrote:
I noticed that you have reworded the commit message: I usually do not > use conjugated verbs in commit headline. Does this conflict with >
flashrom's guidelines?
No formal guidelines, but "128 bytes write granularity support" is missing a crucial piece of information: Was this a bugfix, a new feature or something you removed? The reworded commit message has that info.
Well, it does end with "support", which indicates that support for this was introduced, so I don't think it was confusing.
I really see commit headlines as titles, not active phrases and would like to keep my commits phrased this way.
Please let me know during patch review next time you find the commit headline not precise enough.
Hello Paul,
I have not replied yet because Carl-Daniel's message described the thoughts I had/have when changing commit logs pretty much exactly. Your approach on commit logs (seeing the first line as the "subject") is IMHO a defensible argument. After all, that's how git calls it as well in many places.
However, using imperative sentences is much more common and IMHO it is better. I see every commit as an entity that changes our code base... so it always *does* something and often it is interesting what the most descriptive verb is for what it is doing (e.g. add, fix, improve, refactor, ...) to better understand what the change is about. Just like Carl-Daniel explained for the commit in question.
I can not be sure that if we someone looks at the commit in three years he will remember or even know that 128 byte granularity was introduced about the time this commit happened. It is much easier to understand a commit if the subject contains a verb that makes the action clear.
Also, let's suppose we would have to revert a commit because it was really a bad idea to commit it in the first place and we can not fix it easily. The normal commit message would be «Revert "128 bytes write granularity support"» in your case vs. «Revert "Add support for 128 bytes write granularity.» in the imperative case. The issue with the subject without a verb is similar to the original. In the second case it is completely clear that the support was removed again. Without a verb it could be a fix that is reverted, an improvement, or even the revert of removing the support... it is simply not explicit enough IMHO.
Others have written about the subject as well, but I can not find a lot of texts explaining the WHY very good... this is one example:
http://chris.beams.io/posts/git-commit/#imperative
Your other patches are affected as well and I intend to rephrase them as I see fit. I hope you understand/agree now... if not I'll have another argument: the maintainers have to maintain the codebase and are the ones who read commit messages the most, hence it should be the decision of the maintainers what's written in commit messages because they are affected most by them. The submitter can simply dump the code and hope it gets merged but apart from seeing some mails with the subject during communication it does not affect his work on the project later (because there often is none... otherwise he is not merely a submitter).