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

[Hajime Tazaki]

Hi Mathieu,

At Wed, 15 Apr 2009 09:26:42 +0200,
Mathieu Lacage wrote:
>
>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

Okay, 

- Adding simu_xxx method
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/41044
http://codereview.appspot.com/41043

- fix the problem multiple process stop failure
http://codereview.appspot.com/41045


- Cmsg function enhancement
http://codereview.appspot.com/40065
http://codereview.appspot.com/40064
http://codereview.appspot.com/40063
http://codereview.appspot.com/40062
http://codereview.appspot.com/40061

- clear memory in initialization
http://codereview.appspot.com/40060

- add SocketRecvIfTag and SetIfIndex in socket.cc
http://codereview.appspot.com/40059

next time, I'll try to request better than now.
But maybe I have to be careful to commit in local
repository.

If upload.py can select several changset during post, it's
better..


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

Sorry, the following is that.
http://codereview.appspot.com/40071/show

The revision number between local repo and another repo was
different (I'm not sure). It made a wrong title for issues...

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

I see, you are right.


regards,
hajime

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