[flashrom] [PATCH 1/3] 128 bytes write granularity support

Stefan Tauner stefan.tauner at alumni.tuwien.ac.at
Thu Oct 22 22:05:00 CEST 2015


On Thu, 22 Oct 2015 08:58:53 +0200
Paul Kocialkowski <contact at 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).

-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner




More information about the flashrom mailing list