Skip to content
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

histogram support for ruby client #24

Merged
merged 1 commit into from
Aug 8, 2016
Merged

Conversation

rolandhawk
Copy link
Contributor

Is this acceptable?

ref #21

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 99.589% when pulling 723890a on bukalapak:master into 670c45c on prometheus:master.

# HELP xuq xuq description
xuq{code="ah",le="10"} 1
xuq{code="ah",le="20"} 2
xuq{code="ah",le="30"} 2

Choose a reason for hiding this comment

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

You must expose a +Inf bucket wit the same value as _count

@coveralls
Copy link

coveralls commented Jul 12, 2016

Coverage Status

Coverage increased (+0.03%) to 99.728% when pulling 8a02940 on bukalapak:master into 670c45c on prometheus:master.

@coveralls
Copy link

coveralls commented Jul 12, 2016

Coverage Status

Coverage increased (+0.03%) to 99.729% when pulling ae0af38 on bukalapak:master into 670c45c on prometheus:master.

:histogram
end

def add(labels, value)

Choose a reason for hiding this comment

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

This should be called observe

@grobie
Copy link
Member

grobie commented Aug 3, 2016

Thanks for your contribution! This looks great in general, please only fix the last few comments.

I updated the formatting to the latest rubocop rules (not sure following rubocop was a good idea, they change their mind so often ...). Please rebase and squash your changes against current master. I'll fix the test dependencies for "old" ruby versions tomorrow.

@rolandhawk
Copy link
Contributor Author

Hi @grobie @brian-brazil, I have updated my PR, please kindly check it

@coveralls
Copy link

coveralls commented Aug 8, 2016

Coverage Status

Coverage increased (+0.04%) to 99.733% when pulling f4a4ea9 on bukalapak:master into 58bd969 on prometheus:master.

@grobie
Copy link
Member

grobie commented Aug 8, 2016

Thanks a lot @rolandhawk!

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