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.
- 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.
- 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!
- 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@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware