-
Notifications
You must be signed in to change notification settings - Fork 188
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
feat: export inner MetricVecBuilder structs #490
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: MrCroxx <[email protected]>
In risingwave#9756 I built self-managed metrics generic types with the inner builder types. IMO exporting the inner builder helps a lot. Is It okay to export them to all user or is there any better way to achieve it? Thanks a lot. 🙏 |
You should also bump the MSRV in the CI file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rest LGTM
Seems a new version of indirect dependency requires the new rust version. 🤔 |
@nickelc PTAL 🙏 If this PR is reasonable. Thanks a lot. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@lucab PTAL |
The changes look fine.
The dev dependency
|
@nickelc Thanks for help. Should I pin the dependency to the version that meets the MSRV(1.57) requires or update MSRV to a higher version? |
@MrCroxx thanks for the patch! Let's bump the MSRV, 1.60 sounds like a good target. |
Besides, would you like to publish a new version of this lib after this PR is merged? |
@nickelc Sorry to bother you again. Can this PR be merged? |
@lucab Hi, any updates? Can this PR be merged? |
Signed-off-by: MrCroxx [email protected]
This PR exports
MetricVecBuilder
types, with which users can easily build customized tools.