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

Change BufferedReader.read(while:) signature #2688

Merged
merged 4 commits into from
Mar 20, 2024
Merged

Conversation

gjcairo
Copy link
Contributor

@gjcairo gjcairo commented Mar 20, 2024

Return whether EOF read or predicate doesn't hold anymore in BufferedReader.read(while:).

Motivation:

Using BufferedReader.read(while:) isn't currently too nice to use because we can't differentiate between reading EOF and the predicate not holding anymore. See #2665.

Modifications:

This change deprecates BufferedReader.read(while:) and replaces it with a new version that returns not only the read bytes (as a ByteBuffer) but also a boolean indicating whether we finished reading because EOF has been read, or because the predicate doesn't hold true anymore.

Result:

BufferedReader.read(while:) now returns not only the read bytes (as a ByteBuffer) but also a boolean indicating whether we finished reading because EOF has been read, or because the predicate doesn't hold true anymore.

@gjcairo gjcairo requested a review from glbrntt March 20, 2024 10:42
@gjcairo gjcairo added the 🔼 needs-minor-version-bump For PRs that when merged cause a bump of the minor version, ie. 1.x.0 -> 1.(x+1).0 label Mar 20, 2024
Copy link
Contributor

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

Looks good modulo a couple of nits

Sources/NIOFileSystem/BufferedReader.swift Outdated Show resolved Hide resolved
Sources/NIOFileSystem/BufferedReader.swift Show resolved Hide resolved
@gjcairo gjcairo marked this pull request as ready for review March 20, 2024 11:03
@gjcairo gjcairo enabled auto-merge (squash) March 20, 2024 11:13
@gjcairo gjcairo merged commit d15793c into apple:main Mar 20, 2024
8 of 9 checks passed
@gjcairo gjcairo deleted the read-while branch March 20, 2024 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔼 needs-minor-version-bump For PRs that when merged cause a bump of the minor version, ie. 1.x.0 -> 1.(x+1).0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants