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.

Sign up for GitHub

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

Jump to bottom

Adding ToCounterValue impls #201

Merged
merged 3 commits into from
Mar 9, 2024
Merged

Adding ToCounterValue impls #201

merged 3 commits into from
Mar 9, 2024

Conversation

James-Bartman
Copy link
Contributor

Adding ToCounterValue implementations for i32 and unsigned integers since most counters seem to be unsigned

56quarters
56quarters requested changes Mar 7, 2024
View reviewed changes
Copy link
Owner

@56quarters 56quarters left a comment

Choose a reason for hiding this comment

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

Thanks!

Could you also add to the list of traits that comprise MetricSink here and add some type specific examples in the doc tests?

Also need to add these new types to the tests for the client here and here?

@James-Bartman
Copy link
Contributor Author

Sure thing, just updated them. Let me know if there's anything else I should take care of and thanks for responding so quickly!

56quarters
56quarters approved these changes Mar 9, 2024
View reviewed changes
Copy link
Owner

@56quarters 56quarters left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@56quarters 56quarters merged commit cc0552b into 56quarters:master Mar 9, 2024
6 checks passed
56quarters pushed a commit that referenced this pull request Mar 9, 2024
@James-Bartman @56quarters
Add ToCounterValue and Counted impls for u64 u32 i32 (#201)
63c3268
@James-Bartman James-Bartman deleted the to_counter_value branch March 11, 2024 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@56quarters 56quarters 56quarters approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@James-Bartman @56quarters