Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Rename the waitForFinalResult() methods and provide an ELF convenience #2483

Merged
merged 1 commit into from
Jul 31, 2023

Conversation

FranzBusch
Copy link
Member

Motivation

The name waitForFinalResult() is bad in a couple of ways. First, we normally use wait for blocking operations. Secondly, finalResult is also just strange because the user only wants to know the result and doesn't even know that there might be intermediates.

Modification

This PR renames the waitForFinalResult() method to be a property on NIOProtocolNegotiationResult. Additionally, it provides a conditional extension on ELF to avoid a get().

Result

Better naming for our async interfaces

@FranzBusch FranzBusch force-pushed the fb-rename-protocol-negotiation branch from 0c66748 to 8953d0f Compare July 27, 2023 12:06
@FranzBusch FranzBusch requested a review from glbrntt July 27, 2023 12:06
Copy link
Contributor

@rnro rnro left a comment

Choose a reason for hiding this comment

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

This definitely looks more readable! I'm still not wild about negotiationResult.getResult but I think that's out of scope for this PR

@FranzBusch FranzBusch force-pushed the fb-rename-protocol-negotiation branch from 8953d0f to f96fde0 Compare July 28, 2023 09:12
…ience

# Motivation
The name `waitForFinalResult()` is bad in a couple of ways. First, we normally use `wait` for blocking operations. Secondly, `finalResult` is also just strange because the user only wants to know the _result_ and doesn't even know that there might be intermediates.

# Modification
This PR renames the `waitForFinalResult()` method to be a property on `NIOProtocolNegotiationResult`. Additionally, it provides a conditional extension on `ELF` to avoid a `get()`.

# Result
Better naming for our async interfaces
@FranzBusch FranzBusch force-pushed the fb-rename-protocol-negotiation branch from e8faac7 to 6765dfd Compare July 31, 2023 08:56
@FranzBusch FranzBusch enabled auto-merge (squash) July 31, 2023 08:56
@FranzBusch FranzBusch added 🔨 semver/patch No public API change. 🆕 semver/minor Adds new public API. and removed 🔨 semver/patch No public API change. labels Jul 31, 2023
@FranzBusch FranzBusch merged commit df49d6b into apple:main Jul 31, 2023
@FranzBusch FranzBusch deleted the fb-rename-protocol-negotiation branch July 31, 2023 09:19
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🆕 semver/minor Adds new public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants