[Sound-open-firmware] Commits in SOF

[Guennadi Liakhovetski]

On Tue, May 28, 2019 at 11:23:38AM -0500, Pierre-Louis Bossart wrote:
> 
> 
> On 5/28/19 9:42 AM, Liam Girdwood wrote:
> > On Tue, 2019-05-28 at 17:26 +0300, Daniel Baluta wrote:
> > > Hello all,
> > > 
> > > I have few comments about commits in SOF FW which
> > > I think might improve the review quality.
> > > 
> > > 1) I've seen commit messages that do not explain WHY the
> > > change is needed but mostly saying WHAT the change is
> > > doing.
> > > 
> > > Explaining WHAT the change is doing is good when the code
> > > is fairly complicated. Anyhow, I would prefer to see more of
> > > WHY do we need that change.
> > > 
> > > Lets try to keep this in mind.
> > > 
> > > 2) Commit messages do not contain a verb.
> > > 
> > > Commits should be seen as actions thus should contain
> > > a verb. More than that we should use imperative mood.
> > > 
> > > Not added, fixed, etc but Add, Fix, etc!
> > > 
> > > 3) Capitalize the subject line
> > > 
> > > First letter from commit subject after the subsystem tag should be
> > > capital.
> > > 
> > > e.g. buffer: add new interface -> buffer: Add new interface

Personally I capitalise the subject line only if it doesn't contain
any prefixes, e.g.

37624b5 Linux 5.1-rc7

or

b2a20fd Merge branch 'bnxt_en-Misc-bug-fixes'

etc. I.e. really only special cases. When there are prefixes I
personally optically prefer

8118b82 mm/page_alloc.c: fix never set ALLOC_NOFRAGMENT flag

to

ef61eb4 USB: yurex: Fix protection fault after device removal

As far as I know in English a colon should be followed by lower
case, it's in German that it is followed by upper case. So, I
would appreciate it if we didn't try to enforce this, at least
not until there's a corresponding entry in CodingStyle ;-) We
also could collect some statistics. E.g. of the last 5000 commits
in my current Linux tree, that contain a colon in the subject,
 2692 begin with a lower case after the last colon and 1825 with
upper case.

Thanks
Guennadi

> > > 
> > > I think we should turn our attention to:
> > > 
> > > https://chris.beams.io/posts/git-commit/#seven-rules
> > > 
> > > thanks,
> > > Daniel.
> > > _______________________________________________
> > 
> > 
> > Ack, good points for everyone to use when creating new PRs and patches.
> 
> Yep, more that 50% of my comments for kernel patches are "fix your commit
> messages" and it's even worse for SOF proper.
> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware