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

Consistently use Task or ValueTask in APIs #1646

Merged
merged 1 commit into from
Aug 6, 2024

Conversation

lukebakken
Copy link
Contributor

Fixes #1645

@lukebakken lukebakken self-assigned this Jul 30, 2024
@lukebakken lukebakken added this to the 7.0.0 milestone Jul 30, 2024
@danielmarbach
Copy link
Collaborator

Let's assume the initial design decision was that the ValueTask gets exposed from the bottom layer to the top where the underlying infrastructure like channel is used that already supports ValueTask. If that is true (which I don't know if it is and we might need to ask the original authors of all those changes) then it would make sense to add ValueTask to all the framehandler methods that require it. I haven't looked into details whether it makes sense to do that for close.

Are you following a strategy of doing it "just consistently across the framework handler regardless"?

@lukebakken
Copy link
Contributor Author

Are you following a strategy of doing it "just consistently across the framework handler regardless"?

Yep! I don't know the rationale behind the API now.

@lukebakken
Copy link
Contributor Author

@danielmarbach let me know if there are other places in the library you're looking at with regard to this.

Fixes #1645

* Start with `IFrameHandler` as pointed out by @danielmarbach
@lukebakken lukebakken marked this pull request as ready for review August 6, 2024 19:32
@lukebakken lukebakken merged commit 5c7a139 into main Aug 6, 2024
11 checks passed
@lukebakken lukebakken deleted the rabbitmq-dotnet-client-1645 branch August 6, 2024 19:53
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.

Re-review use of Task vs ValueTask in API
2 participants