Thoughts to keep in mind for Code Review

[Gustavo Niemeyer]

Agreed, but for a slightly different reason. The suggestion is to
annotate the patch with the reason for the change, rather than the
code itself, which might indeed lead to a different kind of comment.
While this might be useful, one of the interesting outcomes of code
reviewing is that it forces the final logic to go through different
eyes and mindsets. The "I don't get it" is not always a bad thing in a
review.. it's rather the reason why simplifications and entirely
different approaches are suggested. Many times I consciously avoid
reading an on-going discussion in the review before doing my own
review, precisely so I can get a fresh perspective on the code before
getting to know everyone else's. Then, with inline reviewing saying
"Please tell me why you did this" is very cheap on both ends.


On Wed, Jun 25, 2014 at 1:42 AM, Ian Booth <ian.booth at canonical.com> wrote:
> -1 on annotations. If you need to annotate to make it clearer then that should
> be done as code comments so the next poor soul who reads the code has a clue of
> what's been done
>
> On 25/06/14 14:20, John Meinel 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



-- 

gustavo @ http://niemeyer.net