Skip to content

Conversation

@vyorkin
Copy link

@vyorkin vyorkin commented Jul 28, 2025

Hi Yoshua,
Thank you for maintaining this crate!
In this PR I tried to carefully upgrade the dependencies to their latest versions and fix some clippy warnings.

Here is the list of updated crates:

  • futures-lite (1.11.3 -> 2.6.0)
  • http-types (2.10.0 -> 2.12.0)
  • log (0.4.8 -> 0.4.27)
  • memchr (2.3.3 -> 2.7.5)
  • pin-project-lite (0.2.7 -> 0.2.16)
  • async-channel (1.1.1 -> 2.5.0)
  • async-std (1.6.0 -> 1.13.1)
  • femme (2.0.0 -> 2.2.1)

I also had to update a few tests to use Box::pin for encoder, like this:
Here is what I mean:

let mut reader = decode(BufReader::new(Box::pin(encoder)));

Hopefully this is the right approach...

I split the changes into multiple commits to make the review easier.
Please let me know if anything needs improvement — I’m happy to help.

pub(crate) fn new(reader: R) -> Lines<R>
where
R: AsyncBufRead + Unpin + Sized,
R: AsyncBufRead + Sized,
Copy link
Author

@vyorkin vyorkin Jul 28, 2025

Choose a reason for hiding this comment

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

I removed Unpin but I'm not sure if this is correct.
Does reader: R actually need to implement Unpin...?
My understanding of pinning is quite limited :(

this.buf.pop();
}
Poll::Ready(Some(Ok(mem::replace(this.buf, String::new()))))
Poll::Ready(Some(Ok(std::mem::take(this.buf))))
Copy link
Author

@vyorkin vyorkin Jul 28, 2025

Choose a reason for hiding this comment

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

std::mem::take is just more concise and idiomatic (clippy suggested this change):
since String implements Default as an empty string, mem::take should replace this.buf with an empty string

Comment on lines -32 to -34
/// Was the last character of the previous line a \r?
/// Bytes that were fed to the decoder but do not yet form a message.
buffer: Vec<u8>,
Copy link
Author

@vyorkin vyorkin Jul 28, 2025

Choose a reason for hiding this comment

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

I didn't find any usage of this buffer field

@vyorkin
Copy link
Author

vyorkin commented Sep 12, 2025

Hi @yoshuawuyts, sorry to bother you — just a quick reminder about PR.
It’s been open for a little while, and I’d really appreciate it if you could take a look whenever you have a moment.
Thanks so much! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant