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.

Sign up for GitHub

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

Jump to bottom

Make EventLoopPromise conform to Equatable #2714

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

Make EventLoopPromise conform to Equatable #2714

wants to merge 1 commit into from

Conversation

gjcairo
Copy link
Contributor

@gjcairo gjcairo commented May 3, 2024

This PR conforms EventLoopPromise to the Equatable protocol.

Motivation:

EventLoopPromise is not currently Equatable, whereas the underlying EventLoopFuture is.
For consistency, it makes sense for EventLoopPromise to conform to Equatable too.

Modifications:

Conform EventLoopPromise to Equatable. This is trivial, as the only property is the underlying EventLoopFuture, which is already Equatable.

Result:

EventLoopPromise is now Equatable.

@gjcairo gjcairo requested a review from Lukasa May 3, 2024 17:18
@Lukasa Lukasa 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 May 7, 2024
Lukasa
Lukasa requested changes May 7, 2024
View reviewed changes
Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

Mind adding some tests here Gus?

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

@Lukasa Lukasa Lukasa requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@gjcairo @Lukasa