Hi Stefan,
Le jeudi 22 octobre 2015 à 22:05 +0200, Stefan Tauner a écrit :
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.
Well, I don't see both points of view as contradictory. When introducing support for something new, I think we're not losing any information when formatting the subject line as: Feature foo support introduction instead of Introduce support for feature foo
This goes for all the things that we might want to describe: * Bug #123 fixup * Feature bar improvement * Baz code re-factoring
Most active verbs have (more or less direct) equivalent nouns that can be used.
I get the argument that we're doing something to the code base, so it makes sense to use active verbs, but for some reason, I just find that somewhat ugly. Perhaps my vision of things will change with time, but that's how I see it for now.
Also, it seems that there is a lot of disparity regarding the tense used for such verbs : Added support for X/Add support for X/Adds support for X. And I don't really see any good way to choose between those. And I hate that sort of lack of consistency, so using non-verbal sentences felt like an elegant solution.
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.
Well, let's not forget that commit messages come with longer explanations that must play a role, more than saying the same as the subject. I really see the subject as a title to the longer commit message.
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.
Well, I think that "support" implies "support introduction", but that's something else. Also, git wraps the original line around quotation marks, which also adds some clarity.
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:
One very good point here is that git uses the imperative when creating a commit on its own (Revert, Merge, etc). That sounds reasonable enough as a guideline for me to switch my behaviour.
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).
Well, that makes sense, yes, but keep in mind that it's also part of the author's contribution, so I'd like things to be discussed beforehand, just like it is somewhat rude to change a patch the way you like and merge it without discussing it with its author through code review first.
Let's say that I wouldn't object to having commit subjects rephrased the way git does it internally.
Thanks for taking the time to explain all this.
Paul Kocialkowski