Ever since I've been using git, I have super great and quick access to
our full history and commit logs.  Being in a different time zone, I
usually warm up in the morning by reading yesterday's commits.  I've
found in general we are very good and strict about review code, but
hardly anyone ever comments on the review description.  I often have
to follow the code review link in the commit, and read through 30
review messages to get the information I wanted.  A few problems I see
very commonly:

- Simple style issues:
Typos / spelling errors.  Wacky line breaks, this is partially
rietveld's fault, but we should standardize on something and fix
rietveld / gcl.

- Bigger issues
A review description that should be a review message.
Commit 1234:
"Make this change.  Please review carefully, what do you think about
the last half?"

A "funny" commit message.  I'm not all dry, I like jokes, but they're
not that funny in 1 year when you're trying to find a regression.

A review description that is completely stale from the code.  When we
iterate on rounds of feedback, we should also comment on the commit
description to make sure it's updated.  Because of the way GCL works,
we do the commit description first, and not last.  There can be a huge
shift in the code between those points, and often the commit message
goes forgotten.

So, here is my proposal.  We review commit descriptions along with
code reviews.  In general, we should think of every commit description
as if it were a comment in the code.  That means proper spelling,
formatting, up to date, etc.  It's important to remember that although
you might be making a quick fix that is obvious to everyone in the
room (hey, the tree was red!), in 2 days no one will remember that
situation.  The code description will be left as 'qucik fix!', when it
instead should have been "Correct a typo in XXX, the previous code
fails to compile."

Are you with me?

Thanks
-- dean

--~--~---------~--~----~------------~-------~--~----~
Chromium Developers mailing list: [email protected] 
View archives, change email options, or unsubscribe: 
    http://groups.google.com/group/chromium-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to