-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade Prometheus client to 1.1 and go-kit to 0.9 #78
Upgrade Prometheus client to 1.1 and go-kit to 0.9 #78
Conversation
Signed-off-by: Yuri Shkuro <ys@uber.com>
Codecov Report
@@ Coverage Diff @@
## master #78 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 26 26
Lines 890 890
=====================================
Hits 890 890 Continue to review full report at Codecov.
|
Signed-off-by: Yuri Shkuro <ys@uber.com>
glide.yaml
Outdated
subpackages: | ||
- metrics/influx | ||
- package: github.com/uber-go/tally | ||
version: '>= 2.1.0, < 4' | ||
- package: github.com/prometheus/client_golang | ||
version: v0.8.0 | ||
version: v1.1.0 |
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 think for glide, you still want a version range rather than an exact version.
^1
should suffice for client_golang and ~0.9.0
for go-kit.
(If I remember correctly, "X.Y.Z" does the right thing in dep so ~
or ^
aren't needed in the Gopkg.toml.)
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.
+1
"github.com/stretchr/testify/assert" | ||
|
||
"github.com/uber/jaeger-lib/metrics" | ||
"github.com/uber/jaeger-lib/metrics/go-kit" | ||
) | ||
|
||
// "github.com/uber/jaeger-lib/vendor/github.com/influxdata/influxdb/client/v2".BatchPointsConfig | ||
// "github.com/uber/jaeger-lib/vendor/github.com/influxdata/influxdb1-client/v2".BatchPointsConfig |
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.
what's up with these comments?
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.
left-over, removing
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 just realized that you'll need to release this at the same time as jaeger-client, otherwise glide will fail with an unsatisfiable constraint: ^1
and == 0.8
aren't compatible.
If you'd like to guard against that, consider changing client_golang to >= 0.8, < 2
as jaeger-lib is compatible with 0.8, 0.9, and 1.x.
Signed-off-by: Yuri Shkuro <ys@uber.com>
@abhinav I don't see an issue with releasing client and lib simultaneously. |
Resolves #77