Move rate limiter from client to jaeger-lib #35
Conversation
Signed-off-by: Isaac Hier <[email protected]>
There was a problem hiding this comment.
I'm ok with moving this over here, however, we can't delete it from jaeger-client-go because it'll be a breaking change since some people maybe using NewRateLimiter. (we could just write a wrapper function NewRateLimiter which just calls this NewRateLimiter...) either way, let's land this but keep client as is for now until we figure out a better migration plan.
| @@ -0,0 +1,77 @@ | |||
| // Copyright (c) 2017 Uber Technologies, Inc. | |||
There was a problem hiding this comment.
although factually these files are just being copied over, shouldn't the copyright be updated here to 2018?
There was a problem hiding this comment.
Right I am wondering too. @yurishkuro what is your opinion here?
There was a problem hiding this comment.
He wrote to me yesterday: "if you copy the file, you keep C as is... if you copy and modify, and C was 2016, you should change it to C 2016, 2018"
There was a problem hiding this comment.
update this and I'll land
There was a problem hiding this comment.
update to what? This is the correct header.
There was a problem hiding this comment.
See my response above from @yurishkuro (granted it is hearsay as he didn't write it here)? It was not modified, so keeping the same.
|
why do we need to move it? It's already a public class, can just use it directly from the go-client path. |
|
You mean the client is accessible from the main jaeger repo or do you mean the client has access to this jaeger-lib repo? I assumed not the former, only the latter. |
|
nvm, I am find to move it. |
|
|
|
I'm still seeing I am not authorized to merge this. |
Signed-off-by: Isaac Hier [email protected]