-
Notifications
You must be signed in to change notification settings - Fork 231
Introduce Clock interface for time measurements #50
Conversation
yurishkuro
commented
Sep 12, 2016
- Use nano-time for measuring duration
- Fix the tests, remove powermock dependency
- add more tests for duration calculation
- Use nano-time for measuring duration - Fix the tests, remove powermock dependency
@adriancole we discussed in the past how this lib used |
} | ||
|
||
@Override | ||
public long currentTimeNanos() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd probably not use the word "Time" here, eventhough nanoTime does. maybe tick or nanoTick? https://google.github.io/guava/releases/19.0/api/docs/com/google/common/base/Ticker.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reason is that people might be confused that it relates to the other "time" in this interface which it doesn't
naming bikeshed aside, looks good. |
@@ -19,11 +19,8 @@ | |||
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN | |||
* THE SOFTWARE. | |||
*/ | |||
package com.uber.jaeger.filters.jaxrs2; | |||
package com.uber.jaeger.context; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oibe these tests weren't in the proper tests dir, so not sure if they were even exercised previously (intelliJ didn't understand them as tests)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted.
lgtm |