[2996] in BarnOwl Developers
Re: Thoughts on our current code review policy
daemon@ATHENA.MIT.EDU (Jason Gross)
Thu Mar 24 01:19:21 2011
In-Reply-To: <alpine.DEB.1.10.1103240003160.25741@dr-wily.mit.edu>
From: Jason Gross <jgross@MIT.EDU>
Date: Thu, 24 Mar 2011 01:18:47 -0400
To: David A Benjamin <davidben@mit.edu>
Cc: Anders Kaseorg <andersk@mit.edu>, Nelson Elhage <nelhage@nelhage.com>,
barnowl-dev@mit.edu
--000e0cd59be26f7870049f339e20
Content-Type: text/plain; charset=ISO-8859-1
2011/3/24 David A Benjamin <davidben@mit.edu>
>
> so I'm not convinced anyone can effectively review changes for e.g.
>>> subtle interactions with other pieces of the code,
>>>
>>
>> This argument cuts both ways: code review is a good way to cause more than
>> one person to be familiar with the code.
>>
>
> I feel like code review is not the best way to get more people familiar
> with code. If I were new, I would not feel comfortable giving a review on
> code I am unfamiliar with and lack context for. And, as a coder, if it were
> subtle code that I really wanted another opinion on, I'm not sure I'm
> looking for such a review either. I guess it's useful for existing
> developers, but I think our problem is much more the headcount than the sum
> of their contents.
>
As a person not that familiar with the code, I often read through the
patches sent out, sometimes somewhat meticulously (and I've found that this
helps me a little with getting familiar with {C,Barnowl}, so I like the fact
that new code gets emailed out. But this is, perhaps, independent of the
code review issue). But I generally don't feel comfortable emailing a
review. ("How do I know if this breaks something else?")
-Jason
--000e0cd59be26f7870049f339e20
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
<br><br><div class=3D"gmail_quote">2011/3/24 David A Benjamin <span dir=3D"=
ltr"><<a href=3D"mailto:davidben@mit.edu" target=3D"_blank">davidben@mit=
.edu</a>></span><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 =
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>
<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p=
x #ccc solid;padding-left:1ex"><blockquote class=3D"gmail_quote" style=3D"m=
argin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
so I'm not convinced anyone can effectively review changes for e.g.<br>
subtle interactions with other pieces of the code,<br>
</blockquote>
<br>
This argument cuts both ways: code review is a good way to cause more than<=
br>
one person to be familiar with the code.<br>
</blockquote>
<br></div>
I feel like code review is not the best way to get more people familiar wit=
h code. If I were new, I would not feel comfortable giving a review on code=
I am unfamiliar with and lack context for. And, as a coder, if it were sub=
tle code that I really wanted another opinion on, I'm not sure I'm =
looking for such a review either. I guess it's useful for existing deve=
lopers, but I think our problem is much more the headcount than the sum of =
their contents.<br>
</blockquote><div>As a person not that familiar with the code, I often read=
through the patches sent out, sometimes somewhat=A0meticulously (and I'=
;ve found that this helps me a little with getting familiar with {C,Barnowl=
}, so I like the fact that new code gets emailed out. =A0But this is, perha=
ps, independent of the code review issue). =A0But I generally don't fee=
l comfortable emailing a review. ("How do I know if this breaks someth=
ing else?")</div>
</div>-Jason
--000e0cd59be26f7870049f339e20--
