Skip to content
This repository was archived by the owner on Apr 1, 2025. It is now read-only.

Add Clear to Meter and Timer #113

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

henning77
Copy link

I needed Clear() on both Meter and Timer to reset these after a unit test. Counter, Histogram, Sample already have Clear so I added equivalent methods.

@freeformz
Copy link

+1 Can we get this merged?

@FZambia
Copy link

FZambia commented Jul 4, 2015

+1 as this should resolve #120

@mihasya
Copy link
Collaborator

mihasya commented Jul 4, 2015

(starting to try and catch up on the backlog)

I am reluctant to accept a change which extends an interface, then immediately proceeds to add implementations which panic to half of the implementations.

Could the method just be added to the implementation you're testing and left out of the interface?

@mihasya
Copy link
Collaborator

mihasya commented Jul 4, 2015

@FZambia I'll comment in the ticket itself, but I wanted to make it clear here too: nothing you've described in #120 sounds like a bug to me.

@mihasya
Copy link
Collaborator

mihasya commented Nov 29, 2015

Bump, @henning77 can you weigh in on the question from July 4th? Thanks!

@henning77
Copy link
Author

@mihasya Clear() panics when called on the snapshot. Which is reasonable and in line with the other Clear() implementations, e.g. in https://github.com/rcrowley/go-metrics/blob/master/histogram.go

@yalay
Copy link

yalay commented Sep 2, 2016

+1 Can we get this merged? Seems to be reasonable.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants