[Ns-developers] Review request (ns-3-simu extension) (Re: About real-world application integration)

[Mathieu Lacage]

hi hajime,

I looked at these patches and posted some reviews for some of them. A
couple of high-level comments:

1) Most of these are too small to be a single review request: you need
to group them logically for me to be able to make sense of them. For
example all these should be a single review request:
http://codereview.appspot.com/40069
http://codereview.appspot.com/40068
http://codereview.appspot.com/41047
http://codereview.appspot.com/40067

2) There are obviously pieces missing from the patchset: where is the
implementation of simu_getopt_long_r ?

Generally, I think that when you (or anyone else) submits a patch, you
have to ask yourself: what would I do if I had to review this patch ? Is
this patch sufficiently self-contained to allow the reviewer to review
it ? Do I need to provide explanation text ? Can I split this patch in
separate self-contained smaller pieces ? i.e., the question is: what can
I do to make it as easy as possible for the reviewer to say 'ACK,
please, apply' ? If you have good answers to these questions, the
chances are good that your patches will go in ns-3-dev really quickly
with few reviews. If you don't, changes are good that it will take a lot
of work and iterations with a reviewer which is really not good for
anyone.

Mathieu

On Wed, 2009-04-15 at 01:24 +0900, Hajime Tazaki wrote:
> Hi All,
> 
> >http://www.sfc.wide.ad.jp/~tazaki/hg/ns-3-simu-mod/
> 
> I post several review requests.
> 
> http://codereview.appspot.com/40071
> http://codereview.appspot.com/41049
> http://codereview.appspot.com/41048
> http://codereview.appspot.com/40070
> http://codereview.appspot.com/40069
> http://codereview.appspot.com/40068
> http://codereview.appspot.com/41047
> http://codereview.appspot.com/40067
> http://codereview.appspot.com/41046
> http://codereview.appspot.com/41045
> http://codereview.appspot.com/40065
> http://codereview.appspot.com/40064
> http://codereview.appspot.com/40063
> http://codereview.appspot.com/41044
> http://codereview.appspot.com/41043
> http://codereview.appspot.com/40062
> http://codereview.appspot.com/40061
> http://codereview.appspot.com/40060
> http://codereview.appspot.com/40059
> 
> regards,
> hajime
>