search

ros / roscpp_core

ros distribution sandbox
89 stars 116 forks source link

fixed minor overflow bugs in Time and Duration #58

Closed subject-name-here closed 7 years ago

dirk-thomas commented 7 years ago

@subject-name-here Thank you for pointing out these issue and proposing a fix. The new tests have been very helpful. The actual code changes to make the tests pass seem a bit "too complicated" to me. Since I couldn't describe why and how it should look like instead I tried to come up with a simpler patch which passes your tests. Please see the proposed changes in #61. Do you agree that #61 also addresses the issue sufficiently and is slightly easier to read / understand?

dirk-thomas commented 7 years ago

I will close this in favor of #61. It would be great if you could still comment here or on #61 about the modified patch. Thanks!

subject-name-here commented 7 years ago

@dirk-thomas Yes, I agree. Your patch is much easier to read and it covers the issue. But I have a little question. It's not very important, but it can cause misunderstanding.