Allow logging as optional feature #26
Conversation
src/log.rs
Outdated
| @@ -0,0 +1,8 @@ | |||
| macro_rules! logc { | |||
There was a problem hiding this comment.
It is a bit repetitive, but I think it might be nicer if you simply re-exported all of the log methods (error, warn, info, debug, trace) in this file. If you did that, then when you go to consume it, you wouldn't have to import both crate::log::logc in addition to log::level (behind a feature gate). You could just import logc and use its corresponding macro methods.
There was a problem hiding this comment.
Sounds good, I'll make that change.
|
In case someone was depending on the logging events in some way, this change shouldn't affect them. Since the logging feature will still be enabled by default. Consumers would need to opt out, via the default-features flag to not have the logging. |
src/tokio/io.rs
Outdated
| @@ -160,16 +160,16 @@ where | |||
| let mut result = Err(e); | |||
|
|
|||
| for (i, duration) in (options.retries_to_attempt_fn)().enumerate() { | |||
| let reconnect_num = i + 1; | |||
| let _reconnect_num = i + 1; | |||
There was a problem hiding this comment.
I do not think these need to be changed to lead with _
There was a problem hiding this comment.
I changed it to that to avoid rustc warnings for unused variables when compiling without logging. Those variables are only used in the log statements. Up to you how you want to handle the warnings, I can remove the underscores if you want
There was a problem hiding this comment.
We shouldn't put the burden on the code here, that should be something covered in the macros itself, perhaps with an allow unused macro in the logging macros you created.
There was a problem hiding this comment.
Should be resolve in 6887535
src/tokio/io.rs
Outdated
| @@ -236,11 +236,11 @@ where | |||
| let future_instant = sleep(next_duration); | |||
|
|
|||
| reconnect_status.attempts_tracker.attempt_num += 1; | |||
| let cur_num = reconnect_status.attempts_tracker.attempt_num; | |||
| let _cur_num = reconnect_status.attempts_tracker.attempt_num; | |||
src/tokio/io.rs
Outdated
| @@ -256,7 +256,7 @@ where | |||
| } | |||
|
|
|||
| fn poll_disconnect(mut self: Pin<&mut Self>, cx: &mut Context) { | |||
| let (attempt, attempt_num) = match &mut self.status { | |||
| let (attempt, _attempt_num) = match &mut self.status { | |||
src/tokio/io.rs
Outdated
| @@ -273,8 +273,8 @@ where | |||
| (self.options.on_connect_callback)(); | |||
| self.underlying_io = underlying_io; | |||
| } | |||
| Poll::Ready(Err(err)) => { | |||
| error!("Connection attempt #{} failed: {:?}", attempt_num, err); | |||
| Poll::Ready(Err(_err)) => { | |||
|
Can you bump patch version? |
|
published |
Request to add logging and the log crate as an optional feature enabled as a default feature to help with backwards compatibility.