On 04/14/2014 12:42 AM, Robert O'Callahan wrote:
On Sat, Apr 12, 2014 at 8:29 AM, Gregory Szorc <g...@mozilla.com> wrote:
I came across the following articles on source control and code review:
* https://secure.phabricator.com/book/phabflavor/article/
recommendations_on_revision_control/
* https://secure.phabricator.com/book/phabflavor/article/
writing_reviewable_code/
* https://secure.phabricator.com/book/phabflavor/article/
recommendations_on_branching/
I think everyone working on Firefox should take the time to read them as
they prescribe what I perceive to be a very rational set of best practices
for working with large and complex code bases.
The articles were written by a (now former) Facebooker and the
recommendations are significantly influenced by Facebook's experiences.
They have many of the same problems we do (size and scale of code base,
hundreds of developers, etc). Some of the pieces on feature development
don't translate easily, but most of the content is relevant.
I would be thrilled if we started adopting some of the recommendations
such as more descriptive commit messages and many, smaller commits over
fewer, complex commits.
As a reviewer one of the first things I do when reviewing a big patch is to
see if I can suggest a reasonable way to split it into smaller patches.
As a reviewer I usually want to see _also_ a patch which contains all the
changes.
Otherwise it can be very difficult to see the big picture.
But sure, having large patches split to smaller pieces may help.
Honestly, I think we're already pretty close to most of those
recommendations, most of the time. "More descriptive commit messages" is
the only recommendation there that is not commonly followed, as far as I
can see.
I always just use the link to the bug, so I haven't seen multiline comment
useful at all.
Mostly they are just annoying, making logs harder to read.
-Olli
Rob
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
