Thoughts to keep in mind for Code Review

Menno Smits menno.smits at canonical.com
Wed Jun 25 08:39:27 UTC 2014


I completely agree with Ian's point about code needing to be
self-explanatory and stand on it's own.

That said, the article mentions that the process of creating review
annotations encourages the author to review their own work in a way that
they may have not done otherwise, eliminating problems before anyone else
even looks at the code. Effectively, a phase of self review happens before
peer review driving the defect rate down and probably making the peer
review more efficient.




On 25 June 2014 19:43, roger peppe <rogpeppe at gmail.com> wrote:

> 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
> >
>
> --
> Juju-dev mailing list
> Juju-dev at lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/juju-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/juju-dev/attachments/20140625/45a0d230/attachment.html>


More information about the Juju-dev mailing list