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

Issue 148810043: ns-3 code review: tv spectrum transmitter

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years ago by ben.cizdziel
Modified:
4 years, 5 months ago
CC:
ns-3-reviews_googlegroups.com
Visibility:
Public.

Description

ns-3 code review: tv spectrum transmitter

Patch Set 1 #

Total comments: 50

Patch Set 2 : ns-3 code review: tv spectrum transmitter rev2 #

Total comments: 20

Patch Set 3 : ns-3 code review: tv spectrum transmitter rev3 #

Total comments: 11

Patch Set 4 : ns-3 code review: tv spectrum transmitter rev4 #

Patch Set 5 : ns-3 code review: tv spectrum transmitter & geographic positions rev5 #

Patch Set 6 : tv spectrum transmitter & geographic positions rev6 #

Patch Set 7 : ns-3 code review: tv spectrum transmitter & geographic positions rev7 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -1 line) Patch
M spectrum.rst View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 6
n.p
Some (most formatting) comments. I don't run the code, but I'll take a closer look ...
4 years, 12 months ago (2014-09-24 14:28:01 UTC) #1
Tom Henderson
In general, a nice first submission. This needs a review/comment from Nicola whether he will ...
4 years, 11 months ago (2014-10-08 13:00:10 UTC) #2
Tom Henderson
some additional comments on the second patch https://codereview.appspot.com/148810043/diff/20001/spectrum.rst File spectrum.rst (right): https://codereview.appspot.com/148810043/diff/20001/spectrum.rst#newcode480 spectrum.rst:480: be created ...
4 years, 10 months ago (2014-11-12 14:34:13 UTC) #3
Tom Henderson
this looks nearly ready to me; well done and well documented. the one thing that ...
4 years, 9 months ago (2014-12-23 15:32:13 UTC) #4
Nicola Baldo
The biggest issue for me is the use of non-static instances of SpectrumModel (see detailed ...
4 years, 7 months ago (2015-02-02 20:35:55 UTC) #5
Tom Henderson
4 years, 7 months ago (2015-02-04 19:16:59 UTC) #6
I agree with the patchset 5 changes and have updated the test repository
accordingly.
Sign in to reply to this message.

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