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

Replace NIOAtomics with swift-atomics #1948

Closed
PeterAdams-A opened this issue Aug 31, 2021 · 4 comments · Fixed by #2204
Closed

Replace NIOAtomics with swift-atomics #1948

PeterAdams-A opened this issue Aug 31, 2021 · 4 comments · Fixed by #2204
Labels
blocked 🔼 needs-minor-version-bump For PRs that when merged cause a bump of the minor version, ie. 1.x.0 -> 1.(x+1).0
Milestone
3.0.0

Comments

@PeterAdams-A
Copy link
Contributor

We should use the standard code now it's available.
@PeterAdams-A PeterAdams-A added the ⚠️ needs-major-version-bump For PRs that when merged cause a bump of the major version, ie. x.0.0 -> (x+1).0.0 label Aug 31, 2021
@Davidde94
Copy link
Contributor

+1

@Davidde94 Davidde94 added this to the 3.0.0 milestone Aug 31, 2021
@weissi
Copy link
Member

weissi commented Sep 1, 2021 •
edited
Loading

@PeterAdams-A this doesn't really need a major. This can be broken into 2 pieces of work:

  1. make NIO itself use swift-atomics
  2. deprecate all NIOAtomics
    Once NIO 3 comes out, the deprecated stuff will be removed anyway.

Optionally (which would allow a lot of code to be deleted), NIOAtomics could also be re-implemented on top of swift-atomics.

@Lukasa
Copy link
Contributor

Lukasa commented Sep 2, 2021

We still can't do this, as we currently support 5.2 but swift-atomics 1.0 supports 5.3 and upward.

@Lukasa Lukasa added blocked 🔼 needs-minor-version-bump For PRs that when merged cause a bump of the minor version, ie. 1.x.0 -> 1.(x+1).0 and removed ⚠️ needs-major-version-bump For PRs that when merged cause a bump of the major version, ie. x.0.0 -> (x+1).0.0 labels Sep 2, 2021
@stevapple stevapple mentioned this issue Jun 24, 2022
Merged
@stevapple
Copy link
Contributor

This is not blocked anymore with the new supporting strategy. Patch is up in #2204.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
blocked 🔼 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
3.0.0
Development

Successfully merging a pull request may close this issue.

Deprecate NIOAtomics in favor of Atomics stevapple/swift-nio
5 participants
@weissi @Lukasa @Davidde94 @stevapple @PeterAdams-A