[Ns-developers] code review tool

[Gustavo Carneiro]

Well, this tool does not make the job any easier for submitters of patches,
just for the reviewer.  And it does not take full advantage of DVCS, for
instance create a temporary branch to fix a bug, maintainer responds,
submitter makes a new commit in the branch to fix the problems, and so on.

That being said, it may not make submitter's job easier, but it does not
seem to make it noticeably harder either, so no objections from me.

But do we need to do this for _all_ patches, or just the medium/large
patches?

2009/4/9 Mathieu Lacage <mathieu.lacage at sophia.inria.fr>

> hi,
>
> Because the number of pending code reviews is growing quickly, and email
> is a pain to deal with them, tom and I would like to get a code review
> tool up and running. A new collegue of mine, Faker looked at
> review-board.org which looks cool in a demo but is a real pain to setup
> correctly and seems to be problematic to use for our mercurial-based
> workflow.
>
> So, I would like to suggest that we use rietveld instead. I uploaded a
> sample patch: http://codereview.appspot.com/37042
> and anyone can review it if they have a google account. If you have a
> google account, you can also post new patches for review:
> http://codereview.appspot.com/new
>
> I used the upload.py script mentioned in the webpage above to create
> this test review patch:
> hg clone http://code.nsnam.org/mathieu/ns-3-simu
> cd ns-3-simu
> # edit README
> hg ci -m "test" README
> upload.py --rev=4583
>
> The above is pretty straightforward which is why I feel it would be nice
> to pick this tool and just go ahead with actually doing code reviews :)
>
> So, I would like to propose that:
> 1) we create a google group ns-3-reviews: if you want to follow through
> email code reviews, this is where you should subscribe
> 2) we ask those who submit code for review/inclusion to submit it
> through rietveld with upload.py  (I believe that the web interface will
> not work with anything but svn repositories but I would be happy to be
> proven wrong) and make sure they CC ns-3-reviews
>
> One thing some might feel concerned about is adding a dependency on an
> externally-maintained server: I personally don't feel bad about this: we
> can always install a copy of rietveld on a server of our own if we are
> unhappy about google's handling of the corereview.appspot.com server.
>
> It would be nice to hear positive or negative comments about this
> proposal.
>
> Mathieu
>
>


-- 
Gustavo J. A. M. Carneiro
INESC Porto, Telecommunications and Multimedia Unit
"The universe is always one step beyond logic." -- Frank Herbert