Conversation
|
hi Dmitry! thank you for raising the PR. I had a quick look and the proposed changes make a lot of sense. I will aim to review in detail over this weekend. |
|
@dkumsh the pipeline failed (looks mostly because of clippy/code format), I always run these commands before commit cargo clippy --no-deps --all-targets --all-features -- -D warnings
cargo fmtThe msrv you would need to bump in Cargo.toml to next min supported version |
d62f132 to
4fee1fe
Compare
|
Hi Tom, I fixed the PR by making MetricsHandle explicitly Send/Sync (with a safety note that backends must be thread‑safe or used strictly single‑threaded). This resolves the LazyLock Sync error from the allocator. I also moved the [release] table to [workspace.metadata.release] to silence the Cargo warning and removed an extra blank line in the Histogram docs that Clippy flagged. |
|
@dkumsh I have finished the review, added few comments; otherwise looks very good |
|
@HaveFunTrading Thanks for the review. I don't see the inline comments yet - could you please resubmit the review so I can address them. |
|
thanks for addressing @dkumsh , I have merged and released new version |
Hi Tom, we are using your crate and I proposes this set of changes, some micro optimizations and code cleanup. Please have a look. I hope you'll like it.