Please tighten up on commit/code review/issue matching

0 views
Skip to first unread message

Mark Birbeck

unread,
Aug 23, 2009, 9:25:03 AM8/23/09
to ubiquity-...@googlegroups.com
Hello all,

I was trying to find some information about some commits that broke
the tests, and during the course of that discovered that people aren't
following all of the steps that we agreed when committing code to
trunk.

Not wishing to pick on Bryan, but I'm going to use his recent commit
as an example, because it's there. :)

(Note that Bryan's code didn't break the tests, this was just
something I came across.)

First, take a look at revision 2924:

<http://code.google.com/p/ubiquity-xforms/source/detail?r=2924>

Reading this you have no idea whether the code being committed went
through a code review.

I would usually assume it did, so I'm not really worried about that aspect.

My concern is that if someone wanted to know anything about the actual
code that was committed -- such as why a particular decision was made,
why something was left out, whether there was debate about a change,
etc. -- they wouldn't know where to look.

Now, let's come at it from the other direction, and look at revision 2914:

<http://code.google.com/p/ubiquity-xforms/source/detail?r=2914>

Here, we see that some code has been discussed, and then given a
positive code review, but we have no idea if it's in the trunk yet. Is
it still to go in, or was it abandoned in favour of another solution?

We've discussed before about ensuring there is a connection between
the code review and the commit to trunk, but in writing this email I
thought it might actually make a useful blog post.

So here are some guidelines on the subject:

<http://webbackplane.com/mark-birbeck/blog/2009/08/link-issues-and-revisions-in-google-code>

And I wonder if people could just double-check any recent commits to
ensure that the circle is squared, so to speak.

Thanks!

Regards,

Mark

--
Mark Birbeck, webBackplane

mark.b...@webBackplane.com

http://webBackplane.com/mark-birbeck

webBackplane is a trading name of Backplane Ltd. (company number
05972288, registered office: 2nd Floor, 69/85 Tabernacle Street,
London, EC2A 4RR)

Rahul Akolkar

unread,
Aug 28, 2009, 1:09:39 PM8/28/09
to ubiquity-...@googlegroups.com
On Sun, Aug 23, 2009 at 9:25 AM, Mark
Birbeck<mark.b...@webbackplane.com> wrote:
>
> Hello all,
>
> I was trying to find some information about some commits that broke
> the tests, and during the course of that discovered that people aren't
> following all of the steps that we agreed when committing code to
> trunk.
>
<snip/>

Just spent a couple of hours doing code forensics for the v0.7 release
notes and I'll second that (I've retained the link to the guidelines
below). Also suggesting we add to commit messages the issue number as
well as a short description -- just having an issue number in plain
text formats such as output of 'svn log' means having to go the
tracker repeatedly to make sense of the numbers. If a short
description is available in the commit message, it will help weed out
things in any such search or study.

-Rahul

Reply all
Reply to author
Forward
0 new messages