[Ns-developers] code review

[Tom Henderson]

(changing subject header slightly to avoid being caught in mailman's filter)

Mathieu Lacage wrote:

> 
> 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 :)

While the above is easy for that simple case, I think it will be helpful
to our contributors if they could also just upload a diff against the
tip of ns-3-dev, or upload a standalone diff.  It's not clear to me that
upload.py has a straightforward way to do that for repositories where
there have been many merges with ns-3-dev in the past.  Maybe we could
wrap upload.py with some other code to automate that, and distribute the
modified file ourselves.

> 
> 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.

I am not concerned about this aspect.

I think we could really use a tool for this, and I would support trying
this unless other people would like to suggest another tool.

Tom