Skip to content

Allow float gauges#39

Merged
vthiery merged 1 commit into
vthiery:masterfrom
messiaen:float-gauge
Aug 23, 2021
Merged

Allow float gauges#39
vthiery merged 1 commit into
vthiery:masterfrom
messiaen:float-gauge

Conversation

@messiaen

@messiaen messiaen commented Aug 16, 2021

Copy link
Copy Markdown
Contributor

@vthiery Currently only int values can be given for gauges due to the functions definitions. I'd like to be able to send floats.

Comment thread include/cpp-statsd-client/StatsdClient.hpp Outdated
Comment thread include/cpp-statsd-client/StatsdClient.hpp Outdated
Comment thread tests/testStatsdClient.cpp Outdated
Comment thread include/cpp-statsd-client/StatsdClient.hpp Outdated
Comment thread include/cpp-statsd-client/StatsdClient.hpp
Comment thread include/cpp-statsd-client/StatsdClient.hpp Outdated

@kevinkreiser kevinkreiser left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the code looks good to me but i defer to @vthiery. also we need to allow CI to run too just to make sure

Comment thread include/cpp-statsd-client/StatsdClient.hpp Outdated

@vthiery vthiery left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Let's just tweak the test a bit (or add one, as you wish), fix the style and then it should be ok 👍

Comment thread include/cpp-statsd-client/StatsdClient.hpp
Comment thread include/cpp-statsd-client/StatsdClient.hpp Outdated
Comment thread tests/testStatsdClient.cpp Outdated
@vthiery

vthiery commented Aug 17, 2021

Copy link
Copy Markdown
Owner

the code looks good to me but i defer to @vthiery. also we need to allow CI to run too just to make sure

Thanks for the review @kevinkreiser ! I allowed the CI to run 👍

Comment thread include/cpp-statsd-client/StatsdClient.hpp
@messiaen

Copy link
Copy Markdown
Contributor Author

Thanks @vthiery and @kevinkreiser!

  • Fixed formatting
  • Added note about gaugePrecision in code examples in README
  • tweaked test.

@messiaen
messiaen requested a review from vthiery August 17, 2021 15:43

@vthiery vthiery left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
@kevinkreiser - want to have another look before I merge it?

@vthiery
vthiery merged commit a06a5b9 into vthiery:master Aug 23, 2021
@dmitrifried

Copy link
Copy Markdown

Hi @vthiery, are you planning on making a new release version with this commit? Right now our cmake dependency will have to point to the commit hash to get the float guages.

@vthiery

vthiery commented Sep 7, 2021

Copy link
Copy Markdown
Owner

Hi @vthiery, are you planning on making a new release version with this commit? Right now our cmake dependency will have to point to the commit hash to get the float guages.

Hi @dmitrifried
I'll release a v2 today or tomorrow. I just need to prepare a quick migration guide since we've been making quite a lot of changes lately.

@vthiery

vthiery commented Sep 7, 2021

Copy link
Copy Markdown
Owner

Hi @vthiery, are you planning on making a new release version with this commit? Right now our cmake dependency will have to point to the commit hash to get the float guages.

Hi @dmitrifried
I'll release a v2 today or tomorrow. I just need to prepare a quick migration guide since we've been making quite a lot of changes lately.

@dmitrifried I just released v2.0.0

@dmitrifried

dmitrifried commented Sep 7, 2021

Copy link
Copy Markdown

@vthiery Thanks for the quick work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants