Skip to content

Remove reliance on the NIO umbrella module #60

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
Merged
merged 5 commits into from
Jun 19, 2023
Merged

Remove reliance on the NIO umbrella module #60

merged 5 commits into from
Jun 19, 2023

Conversation

Joannis
Copy link
Member

@Joannis Joannis commented Jun 16, 2023

No description provided.

@Joannis Joannis requested a review from fabianfett June 16, 2023 22:15
fabianfett
fabianfett reviewed Jun 17, 2023
View reviewed changes
Copy link
Member

@fabianfett fabianfett left a comment

Choose a reason for hiding this comment

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

This looks great. Two things left:

  1. We should change Package.swift and depend on the explicit NIOxxx modules. However we should keep depending on NIO for the time being. See AHC.
  2. We should comment in the don't import NIO check in the soundness script.

@Joannis Joannis requested a review from fabianfett June 18, 2023 11:52
fabianfett
fabianfett reviewed Jun 18, 2023
View reviewed changes
Package.swift
.product(name: "NIOPosix", package: "swift-nio"),
.product(name: "NIOConcurrencyHelpers", package: "swift-nio"),
.product(name: "Atomics", package: "swift-atomics"),
.product(name: "NIO", package: "swift-nio"),
Copy link
Member

Choose a reason for hiding this comment

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

Can we keep the NIO umbrella import for now. Users might depend on it... But we should not use it anywhere.

@fabianfett fabianfett added the 🔨 semver/patch No public API change. label Jun 18, 2023
@Joannis Joannis linked an issue Jun 18, 2023 that may be closed by this pull request
Remove all NIO umbrella imports #59
Closed
fabianfett
fabianfett approved these changes Jun 19, 2023
View reviewed changes
Copy link
Member

@fabianfett fabianfett left a comment

Choose a reason for hiding this comment

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

Thank you!

@fabianfett fabianfett merged commit 2f3db6e into main Jun 19, 2023
@fabianfett fabianfett deleted the jo/remove-nio-umbrella branch June 19, 2023 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fabianfett fabianfett fabianfett approved these changes

Assignees

No one assigned

Labels

🔨 semver/patch No public API change.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove all NIO umbrella imports

2 participants

@Joannis @fabianfett