Thoughts to keep in mind for Code Review

roger peppe rogpeppe at gmail.com
Wed Jun 25 07:43:03 UTC 2014


That's very interesting; thanks for the link.

One part that jumps out at me:

: ... many teams that review code don't have a good way of tracking
defects found
: during review and ensuring that bugs are actually fixed before the
review is complete

We don't have this, and with github reviews it's now even harder (for reviewer
or coder) to verify this by going back to try to correlate code review
remarks with
changes made.

About pre-review annotations, I agree with Ian that the code should be
documented
well enough that someone coming to it from scratch can understand it, but
I also wonder if there is a room for review-specific comments, talking about
reasons for the changes themselves in the specific context of that review.
The code review does not show all the code in the system, and the changes
made will often be made with respect to other areas of the code base - I can see
how a guide to how the changes fit within the context of the whole system,
and *why* we're making the changes themselves, rather than how the changed
code functions when the changes are made, might be a useful thing.

  cheers,
    rog.


On 25 June 2014 05:20, John Meinel <john at arbash-meinel.com> wrote:
> An interesting article from IBM:
> http://www.ibm.com/developerworks/rational/library/11-proven-practices-for-peer-review/
>
> There is a pretty strong bias for "we found these results and look at how
> our tool makes it easier to follow these guidelines", but the core results
> are actually pretty good.
>
> I certainly recommend reading it and keeping some of it in mind while you're
> both coding and reviewing. (Particularly how long should code review take,
> and how much code should be put up for review at a time.)
> Another trick that we haven't made much use of is to annotate the code we
> put up for review. We have the summary description, but you can certainly
> put some inline comments on your own proposal if you want to highlight areas
> more clearly.
>
> John
> =:->
>
> --
> Juju-dev mailing list
> Juju-dev at lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/juju-dev
>



More information about the Juju-dev mailing list