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

Remove conflicting copy of NIO method #141

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

Conversation

Lukasa
Copy link

@Lukasa Lukasa commented Oct 25, 2024

In #98, @tachyonics added a vendored copy of a NIO method on EventLoopFuture to avoid needing to add availability guards. I don't think that should have been necessary then, but it certainly isn't now, as smoke-http's minimum deployment targets match the extension in NIO.

This extension contravened one of NIO's requirements on its dependencies: https://github.com/apple/swift-nio/blob/main/docs/public-api.md#4-extending-nio-types. Ideally when it was added, it should have been given a different name.

This extension has been working, but it is now conflicting with NIO's main on Swift 5.9. This patch removes the now-vestigial extension, as it isn't needed.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

In amzn#98, @tachyonics added a vendored copy of a NIO method on
EventLoopFuture to avoid needing to add availability guards. I don't
think that should have been necessary then, but it certainly isn't now,
as smoke-http's minimum deployment targets match the extension in NIO.

This extension contravened one of NIO's requirements on its dependencies:
https://github.com/apple/swift-nio/blob/main/docs/public-api.md#4-extending-nio-types.
Ideally when it was added, it should have been given a different name.

This extension has been working, but it is now conflicting with NIO's main on
Swift 5.9. This patch removes the now-vestigial extension, as it isn't needed.
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.

1 participant