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

fix visibility for Counter struct methods #38

Merged
merged 1 commit into from
Jan 24, 2021

Conversation

fanatid
Copy link
Contributor

@fanatid fanatid commented Jan 24, 2021

Methods in impl Counter should have the same visibility.

@tobz
Copy link
Member

tobz commented Jan 24, 2021

Thanks for the contribution, and good catch! Just rerunning CI so that it hopefully passes.

Note to self: we probably need/want a test case that somehow forces this codepath to be hit so we can assert that it panics... but I'm not super sure how to do so for a single test case? Like we could override the target architecture/supported CPU features, I think?, but only for the whole run. I guess maybe that + a configuration flag to only run the specific test. Hmm...

@fanatid
Copy link
Contributor Author

fanatid commented Jan 24, 2021

Stil failed CI 😞

@tobz
Copy link
Member

tobz commented Jan 24, 2021

Looks like the actual failure (the previous run just timed out, not a true failure) is related to the benchmark step not being able to save the results to GH Pages... but all tests pass correctly. We can go ahead with merging this as such.

@tobz tobz merged commit 119cde7 into metrics-rs:main Jan 24, 2021
@fanatid fanatid deleted the fix-counter-struct branch January 24, 2021 18:05
@fanatid
Copy link
Contributor Author

fanatid commented Jan 24, 2021

Thanks! Can you also make a release with the patch?

@tobz
Copy link
Member

tobz commented Jan 24, 2021

Yep, just did: [email protected].

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.

2 participants