Fix clippy lints and expose dependencies of API#34
Conversation
hyper::Uri and chrono::{DateTime, Utc} are used in the public Rust API,
so we need to `pub use` them so consumers can use these types and for
them to be compatible.
0.1.4 uses const new, which requires Rust 1.57+
ivoanjo
left a comment
ivoanjo
left a comment
There was a problem hiding this comment.
Left two noob questions, but other than the gitlab-ci this looks to be in good shape :)
| DOWNSTREAM_BRANCH: | ||
| value: "main" | ||
| value: "levi/rust-1.56" | ||
| description: "downstream jobs are triggered on this branch" |
There was a problem hiding this comment.
I'm guessing this change should not go into the main branch :)
There was a problem hiding this comment.
Correct -- this will get changed once the downstream PR is fixed and merged. Moved back to draft to reflect this.
| fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> { | ||
| self.tcp.poll_ready(cx).map_err(|e| e.into()) | ||
| self.tcp.poll_ready(cx) |
There was a problem hiding this comment.
My weak rust-foo is failing me here. Why is it ok to remove the error handling here now and but seems like it was needed before?
There was a problem hiding this comment.
It's because the result of self.tcp.poll_ready(cx) is already type Poll<Result<(), Self::Error>> -- the code .map_err(|e| e.into()) is redundant.
There was a problem hiding this comment.
Thanks... To be honest I search the docs and could not confirm that was the type. Do you have any tips for looking up the types in these cases? Do you just use an IDE for that?
There was a problem hiding this comment.
In this case, clippy told me about it and I verified it in CLion, which is the IDE I use for C and Rust projects.
There was a problem hiding this comment.
Guess it's time I setup CLion for libddprof >_>
| if let Some(path) = str.strip_prefix("unix://") { | ||
| return Ok(ddprof_exporter::socket_path_to_uri(path.as_ref())?); | ||
| return ddprof_exporter::socket_path_to_uri(path.as_ref()); |
There was a problem hiding this comment.
I'm curious -- why is the Ok not needed anymore? Is it assumed as the default on a return if you don't state it anywhere?
There was a problem hiding this comment.
socket_path_to_uri already returns a Result -- there's no need to use ? and wrap it back into Ok ^_^
* Remove unused Exporter APIs and adjust Slices CharSlice -> intended to be valid utf-8 string. ByteSlice -> not intended to be valid utf-8 string (like binary data). * Fix quoting for CMake * Fix exporter example, address CR feedback
2 participants
What does this PR do?
Fix clippy lints and expose dependencies of API.
Motivation
hyper::Uri and chrono::{DateTime, Utc} are used in the public Rust API,
so we need to
pub usethem so consumers can use these types and forthem to be compatible.
Additional Notes
I don't have my POC that's using this API fully working yet, so there may be other needed changes, but at least these should be made.