Skip to content
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

refactor!: Reimplement stream without split #274

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jakoschiko
Copy link
Collaborator

No description provided.

@jakoschiko jakoschiko force-pushed the jakoschiko_stream-without-split branch 5 times, most recently from 8640072 to 4e03c47 Compare September 8, 2024 23:39
@coveralls
Copy link
Collaborator

coveralls commented Sep 8, 2024

Pull Request Test Coverage Report for Build 10764106165

Details

  • 36 of 54 (66.67%) changed or added relevant lines in 2 files are covered.
  • 32 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+4.4%) to 85.406%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/stream.rs 34 52 65.38%
Files with Coverage Reduction New Missed Lines %
src/stream.rs 32 54.81%
Totals Coverage Status
Change from base Build 10762830153: 4.4%
Covered Lines: 1030
Relevant Lines: 1206

💛 - Coveralls

@jakoschiko jakoschiko force-pushed the jakoschiko_stream-without-split branch 2 times, most recently from fe3db17 to 94d5840 Compare September 8, 2024 23:47
@jakoschiko jakoschiko force-pushed the jakoschiko_stream-without-split branch from 94d5840 to 119629b Compare September 8, 2024 23:58
@soywod
Copy link
Contributor

soywod commented Nov 1, 2024

Any news about this refactor? I may need sth similar for time-lib, where a TcpStream is used for both reading and writing, and compatible with tokio or async-std.

@jakoschiko
Copy link
Collaborator Author

If it's blocking you, then I'll continue to work on this.

How do think compatibility with async-std can be achieved? Currently this PR proposed to depend on tokio::io::AsyncRead and tokio::io::AsyncWrite. So I think it's not compatible with async-std.

@soywod
Copy link
Contributor

soywod commented Nov 1, 2024

If it's blocking you, then I'll continue to work on this.

No don't worry, it's not blocking me. Yet it could be a great addition for few libs of mine, including imap-client.

How do think compatibility with async-std can be achieved? Currently this PR proposed to depend on tokio::io::AsyncRead and tokio::io::AsyncWrite. So I think it's not compatible with async-std. For now it's uggly but it can server as a basis.

In time-lib I used futures::Async{Read,Write}. I just needed to write some compatibility layer for tokio and async-std.

I liked your Sans IO approach, I don't know if it could be applicable here, for stream. By default only the stream is exposed, then every feature could expose an IO implementation like tokio-native-tls, or async-std-rustls etc.

@soywod
Copy link
Contributor

soywod commented Nov 1, 2024

Did you initiate a dedicated repo for the stream, as you discussed once? If not then I can definitely start it and invite you guys.

@jakoschiko
Copy link
Collaborator Author

I created the repo tokio-maybe-tls. I'll invite you once the CI is ready.

@jakoschiko
Copy link
Collaborator Author

TIL https://docs.rs/tokio-util/latest/tokio_util/compat exists

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants