Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1192)

Issue 204055: Topology read system (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 1 month ago by Tommaso Pecorella
Modified:
12 years, 11 months ago
CC:
ns-3-reviews_googlegroups.com
Visibility:
Public.

Patch Set 1 #

Total comments: 7

Patch Set 2 : changes according to Pavel's comments #

Total comments: 2

Patch Set 3 : More changes according to Pavel's comments #

Total comments: 2

Patch Set 4 : Complete patch for Josh #

Total comments: 3

Patch Set 5 : Fixed (almost) all the comments from mathieu, Josh and George #

Patch Set 6 : The Node Attributes can be delayed until they'll be actually used. #

Patch Set 7 : added TopologyLinksIterator #

Total comments: 10

Patch Set 8 : Latest comments from Mathieu - fixed #

Total comments: 7

Patch Set 9 : Reuploaded patch since I forgot some bits in the previous one (deleted). #

Patch Set 10 : Call me a fool. I added the Helper. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+11812 lines, -0 lines) Patch
A examples/topology-read/Inet_small_toposample.txt View 3 1 chunk +20 lines, -0 lines 0 comments Download
A examples/topology-read/Inet_toposample.txt View 3 1 chunk +7826 lines, -0 lines 0 comments Download
A examples/topology-read/Orbis_toposample.txt View 3 1 chunk +2769 lines, -0 lines 0 comments Download
A examples/topology-read/topology-example-sim.cc View 1 3 4 5 6 7 8 9 1 chunk +209 lines, -0 lines 0 comments Download
A examples/topology-read/waf View 1 chunk +1 line, -0 lines 0 comments Download
A examples/topology-read/wscript View 1 chunk +5 lines, -0 lines 0 comments Download
A src/contrib/topology-read/inet-topology-reader.h View 1 2 3 4 5 6 7 8 1 chunk +76 lines, -0 lines 0 comments Download
A src/contrib/topology-read/inet-topology-reader.cc View 1 2 3 4 5 6 7 8 1 chunk +140 lines, -0 lines 0 comments Download
A src/contrib/topology-read/orbis-topology-reader.h View 1 2 3 4 5 6 7 8 1 chunk +73 lines, -0 lines 0 comments Download
A src/contrib/topology-read/orbis-topology-reader.cc View 1 2 3 4 5 6 7 8 1 chunk +121 lines, -0 lines 0 comments Download
A src/contrib/topology-read/topology.h View 1 chunk +32 lines, -0 lines 0 comments Download
A src/contrib/topology-read/topology-reader.h View 5 6 7 8 1 chunk +203 lines, -0 lines 0 comments Download
A src/contrib/topology-read/topology-reader.cc View 5 6 7 8 1 chunk +169 lines, -0 lines 0 comments Download
A src/contrib/topology-read/wscript View 1 4 1 chunk +17 lines, -0 lines 0 comments Download
A src/helper/topology-reader-helper.h View 1 chunk +66 lines, -0 lines 0 comments Download
A src/helper/topology-reader-helper.cc View 1 chunk +80 lines, -0 lines 0 comments Download
M src/helper/wscript View 2 chunks +2 lines, -0 lines 0 comments Download
M src/wscript View 9 1 chunk +1 line, -0 lines 0 comments Download
M test.py View 3 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 37
Pavel Boyko
Hi, Tommasso, I really like the idea and enjoy your simple design & implementation. Please ...
14 years, 1 month ago (2010-02-08 06:48:49 UTC) #1
Tommaso Pecorella
Hi Pavel, thank you for checking the code. namespaces: good point, I'll remove them from ...
14 years, 1 month ago (2010-02-08 13:57:27 UTC) #2
Tommaso Pecorella
http://codereview.appspot.com/204055/diff/1/15 File src/contrib/topology-read/topology-reader-interface.h (right): http://codereview.appspot.com/204055/diff/1/15#newcode32 src/contrib/topology-read/topology-reader-interface.h:32: using namespace std; On 2010/02/08 06:48:49, Pavel Boyko wrote: ...
14 years, 1 month ago (2010-02-09 01:54:18 UTC) #3
Tommaso Pecorella
14 years, 1 month ago (2010-02-09 01:55:33 UTC) #4
Tommaso Pecorella
Great. Another file uploaded by mistake due to ultra-zelant check-style.py. We should use it on ...
14 years, 1 month ago (2010-02-09 01:58:41 UTC) #5
tpecorella_mac.com
Yo all, I need a +1 on this patch, or more comments to fix it ...
14 years, 1 month ago (2010-02-20 11:45:41 UTC) #6
Tommaso Pecorella
Yo all, I need a +1 on this patch, or more comments to fix it ...
14 years, 1 month ago (2010-02-20 14:42:18 UTC) #7
Pavel Boyko
Well, the main remaining issue is to convert examples to tests. I will not vote ...
14 years ago (2010-02-21 10:33:53 UTC) #8
Tommaso Pecorella
Thanks for your reply. I was indeed waiting for feedbacks about the two parts (module ...
14 years ago (2010-02-21 14:35:14 UTC) #9
Tommaso Pecorella
14 years ago (2010-02-21 14:52:41 UTC) #10
Tommaso Pecorella
http://codereview.appspot.com/204055/diff/4008/4014 File examples/topology-read/topology-example-sim.cc (right): http://codereview.appspot.com/204055/diff/4008/4014#newcode106 examples/topology-read/topology-example-sim.cc:106: std::cout << "trying to read " << input << ...
14 years ago (2010-02-22 00:12:04 UTC) #11
Tommaso Pecorella
14 years ago (2010-02-26 13:35:40 UTC) #12
Mathieu Lacage
I did not go through the details but my main suggestions would be something along ...
14 years ago (2010-02-26 15:16:51 UTC) #13
Josh Pelkey
Hi Tommaso, George Riley and I just looked over this. Overall we are happy with ...
14 years ago (2010-02-26 19:42:29 UTC) #14
Tommaso Pecorella
Thanks Mathieu, Josh and George for the valuable comments. I'll start with Mathieu's points. On ...
14 years ago (2010-02-27 03:08:51 UTC) #15
Tommaso Pecorella
14 years ago (2010-02-27 11:30:26 UTC) #16
Josh Pelkey
Considering your latest patch and your comments on our last remaining issue about TopologyLink, +1 ...
14 years ago (2010-02-27 19:56:45 UTC) #17
Mathieu Lacage
On Sat, Feb 27, 2010 at 4:08 AM, <tommypec@gmail.com> wrote: >> 2) change TopologyLink to ...
14 years ago (2010-03-01 09:38:52 UTC) #18
Tommaso Pecorella
Hi Mathieu, I'll try to address your points, that indeed are good points, mind. I ...
14 years ago (2010-03-01 10:21:53 UTC) #19
Tommaso Pecorella
14 years ago (2010-03-02 17:28:39 UTC) #20
Tommaso Pecorella
After a discussion with Mathieu (thanks again) I decided that the whole node attributes thing ...
14 years ago (2010-03-02 17:34:44 UTC) #21
Tommaso Pecorella
14 years ago (2010-03-02 22:43:25 UTC) #22
Tommaso Pecorella
I think this it more or less complete. The iterators are there (only one since ...
14 years ago (2010-03-02 22:50:46 UTC) #23
Mathieu Lacage
comments below. http://codereview.appspot.com/204055/diff/7045/8056 File src/contrib/topology-read/topology-reader.h (right): http://codereview.appspot.com/204055/diff/7045/8056#newcode54 src/contrib/topology-read/topology-reader.h:54: std::map<std::string, std::string > linkAttr; I really do ...
14 years ago (2010-03-04 08:09:01 UTC) #24
Tommaso Pecorella
14 years ago (2010-03-05 02:10:57 UTC) #25
Tommaso Pecorella
What can I say beside the fact that I agree? I like to be pushed ...
14 years ago (2010-03-05 02:23:54 UTC) #26
Josh Pelkey
Mathieu, With merge deadline looming, could you have a look at Tommaso's updated patch set?
14 years ago (2010-03-08 19:54:44 UTC) #27
Mathieu Lacage
More comments. Mostly minor. This is pushing the merge deadline a bit... http://codereview.appspot.com/204055/diff/10001/11007 File src/contrib/attribute-default-iterator.cc ...
14 years ago (2010-03-09 07:43:15 UTC) #28
Tommaso Pecorella
14 years ago (2010-03-09 18:39:19 UTC) #29
Tommaso Pecorella
On 2010/03/09 07:43:15, Mathieu Lacage wrote: > More comments. Mostly minor. This is pushing the ...
14 years ago (2010-03-09 18:58:41 UTC) #30
Tommaso Pecorella
14 years ago (2010-03-09 21:59:20 UTC) #31
tomh_tomh.org
> If we choose to not go for the 3.8 release, I'll do it when ...
14 years ago (2010-03-10 06:36:09 UTC) #32
Mathieu Lacage
On Tue, Mar 9, 2010 at 7:58 PM, <tommypec@gmail.com> wrote: > Yes it is. To ...
14 years ago (2010-03-10 08:00:02 UTC) #33
Tommaso Pecorella
Hi Tom, it's not a big issue after all. It will give me the time ...
14 years ago (2010-03-10 11:52:31 UTC) #34
Josh Pelkey
> I'm surprised to see the door close on this contribution, since it is > ...
14 years ago (2010-03-10 13:00:27 UTC) #35
Tommaso Pecorella
14 years ago (2010-03-10 22:17:38 UTC) #36
Josh Pelkey
14 years ago (2010-03-12 19:53:38 UTC) #37
I just merged this with ns-3-dev: changeset b5bc10de166d

Thanks Tommaso!
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b