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

Issue 14632043: Introduce additional time units (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 3 months ago by Vedran Miletić
Modified:
8 years, 1 month ago
CC:
ns-3-reviews_googlegroups.com
Visibility:
Public.

Description

Introduce additional time units

Patch Set 1 #

Total comments: 6

Patch Set 2 : Updated patch set per Peter and Tom's comments #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+467 lines, -12 lines) Patch
M src/core/model/nstime.h View 1 10 chunks +398 lines, -7 lines 5 comments Download
M src/core/model/time.cc View 1 4 chunks +53 lines, -5 lines 0 comments Download
M src/core/test/time-test-suite.cc View 1 1 chunk +16 lines, -0 lines 0 comments Download

Messages

Total messages: 12
Peter Barnes
Hello Vedran, When you tossed this into the bug954 discussion I thought it would be ...
8 years, 3 months ago (2013-10-18 18:25:56 UTC) #1
Vedran Miletić
On 2013/10/18 18:25:56, Peter Barnes wrote: > Hello Vedran, > > When you tossed this ...
8 years, 3 months ago (2013-10-21 07:51:17 UTC) #2
Vedran Miletić
On 2013/10/21 07:51:17, Vedran Miletić wrote: > On 2013/10/18 18:25:56, Peter Barnes wrote: > > ...
8 years, 3 months ago (2013-10-21 09:03:22 UTC) #3
Vedran Miletić
On 2013/10/21 09:03:22, Vedran Miletić wrote: > On 2013/10/21 07:51:17, Vedran Miletić wrote: > > ...
8 years, 3 months ago (2013-10-21 13:23:06 UTC) #4
Tom Henderson
On 2013/10/21 13:23:06, Vedran Miletić wrote: > On 2013/10/21 09:03:22, Vedran Miletić wrote: > > ...
8 years, 2 months ago (2013-11-22 17:17:50 UTC) #5
Vedran Miletić
Updated, rebased on ns-3-dev (no changes needed though). Year is now 365 days, and week ...
8 years, 1 month ago (2013-11-24 13:20:36 UTC) #6
Peter Barnes
I've played with alternative implementations of the unit conversion code and seen the same thing. ...
8 years, 1 month ago (2013-11-25 20:23:20 UTC) #7
Peter Barnes
https://codereview.appspot.com/14632043/diff/63001/src/core/model/nstime.h File src/core/model/nstime.h (right): https://codereview.appspot.com/14632043/diff/63001/src/core/model/nstime.h#newcode89 src/core/model/nstime.h:89: Y = 0, //!< year //!< Julian calendar year, ...
8 years, 1 month ago (2013-11-25 20:23:31 UTC) #8
Vedran Miletić
On 2013/11/25 20:23:31, Peter Barnes wrote: > https://codereview.appspot.com/14632043/diff/63001/src/core/model/nstime.h > File src/core/model/nstime.h (right): > > https://codereview.appspot.com/14632043/diff/63001/src/core/model/nstime.h#newcode89 ...
8 years, 1 month ago (2013-12-02 22:40:09 UTC) #9
Vedran Miletić
On 2013/11/25 20:23:20, Peter Barnes wrote: > I've played with alternative implementations of the unit ...
8 years, 1 month ago (2013-12-02 22:41:14 UTC) #10
Tom Henderson
On 2013/11/25 20:23:20, Peter Barnes wrote: > I've played with alternative implementations of the unit ...
8 years, 1 month ago (2013-12-03 15:19:50 UTC) #11
Vedran Miletić
8 years, 1 month ago (2013-12-06 18:43:59 UTC) #12
Overloaded constructors are the problem. Removed them; no fails. Apparently we
have some code that relies on some automatic int->float or float->int
conversion.

I removed overloads, added the documentation suggestions made by Peter and
pushed, since I consider this now to be no worse than we had in 3.18.

We can look into fixing problematic code later, and then add overloads if they
are needed.
Sign in to reply to this message.

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