On 03/10/2017 12:43 AM, Ben Kelly wrote:
On Thu, Mar 9, 2017 at 5:35 PM, Mike Hommey <[email protected]> wrote:
On Thu, Mar 09, 2017 at 02:46:53PM -0500, Ehsan Akhgari wrote:
I review a large number of patches on a typical day, and usually I have
to
spend a fair amount of time to just understand what the patch is doing.
As
the patch author, you can do a lot to help make this easier by *writing
better commit messages*. Starting now, I'm going to try out a new
practice
for a while: I'm going to first review the commit message of all patches,
and if I can't understand what the patch does by reading the commit
message
before reading any of the code, I'll r- and ask for another version of
the
patch.
Sometimes, the commit message does explain what it does in a sufficient
manner, but finding out why requires reading the bug, assuming it's
written there. I think this information should also be in the commit
message.
(Just continuing the thread here.)
Personally I prefer looking at the bug for the full context and single
point of truth. Also, security bugs typically can't have extensive commit
messages and moving a lot of context to commit messages might paint a
target on security patches.
Exactly. In practice I tend to just read the first line of a commit message and
the most important part there is the bug number.
I've never been a fan of overlong commit messages.
MozReview makes this a bit different since it decouples bugs and changes, so
there seeing more explanation in the commit message is more useful.
_______________________________________________
dev-platform mailing list
[email protected]
https://lists.mozilla.org/listinfo/dev-platform
