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

Set Swift 5.8 as the minimum supported version #146

Merged
merged 2 commits into from
Oct 21, 2024

Conversation

3a4oT
Copy link
Contributor

@3a4oT 3a4oT commented Aug 29, 2024

  • Set Swift 5.8 as the minimum supported version.
  • Removed CI job for Ubuntu 20.04.
  • Bumped swift-nio and swift-collections versions on the manifest.

@NeedleInAJayStack
Copy link
Member

Thanks @3a4oT Just wondering - Are there any particular needs you had for upgrading this, or were you just cleaning it up?

Package.swift Outdated Show resolved Hide resolved
@NeedleInAJayStack
Copy link
Member

@paulofaria Do you have any thoughts on this? I know you've been hesitant to update swift-tools versions in the past, but NIO is at 5.8 compatibility

@3a4oT
Copy link
Contributor Author

3a4oT commented Sep 5, 2024

Thanks @3a4oT Just wondering - Are there any particular needs you had for upgrading this, or were you just cleaning it up?

Hello, thanks for asking. The main driver was that we should support what Swift-NIO supports regarding the minimum required Swift tools. Swift itself doesn't support Ubuntu 20.04 anymore, so I dropped 5.4-5.6. I thought that bumping the version to 5.8 would be too aggressive, but I still believe it's the right thing to do :)

@NeedleInAJayStack
Copy link
Member

Hey @3a4oT, I think this is good to go if you just make the following changes:

  1. Remove dependency changes in Package.swift
  2. Run the formatter: docker run --rm -v ./:/repo ghcr.io/nicklockwood/swiftformat:latest /repo

Let me know if you need any help!

@paulofaria
Copy link
Member

Let's follow SwiftNIO compatibility.

@3a4oT
Copy link
Contributor Author

3a4oT commented Oct 16, 2024

Hey @3a4oT, I think this is good to go if you just make the following changes:

  1. Remove dependency changes in Package.swift
  2. Run the formatter: docker run --rm -v ./:/repo ghcr.io/nicklockwood/swiftformat:latest /repo

Let me know if you need any help!

Thanks! I have applied your suggestions to the rebased commit.

Let's follow SwiftNIO compatibility.

Set swift-tools version to 5.8.

UPDATED: it seems like swift-nio is about to drop Swift 5.8 support at apple/swift-nio#2924

@3a4oT 3a4oT changed the title Set Swift 5.7 as the minimum supported version Set Swift 5.8 as the minimum supported version Oct 16, 2024
@paulofaria
Copy link
Member

We can merge this with 5.8 and then do a follow up PR with 5.9 or we can just go with 5.9, wait and synchronize with the other PR. I'm fine either way, it's your call. 🙂

By the way, I think we should be explicit about our versioning support. The main question is do we phrase it as "the three latest released Swift versions" or do we phrase it as "we follow SwiftNIO support"? @NeedleInAJayStack

Whatever we decide in this discussion, can you please add it to the Readme? @3a4oT

@NeedleInAJayStack
Copy link
Member

I'm okay merging this and dropping 5.8 after NIO does.

By the way, I think we should be explicit about our versioning support. The main question is do we phrase it as "the three latest released Swift versions" or do we phrase it as "we follow SwiftNIO support"? @NeedleInAJayStack

I agree. I think phrasing it as "we follow SwiftNIO support" makes sense at the moment while we're pretty reliant on NIO. If at some point we drop that dependency in favor of full Swift Concurrency we can reevaluate.

@3a4oT Could you add a Support section to the README between Encoding Results and Contributing that says:

This package supports Swift versions in [alignment with Swift NIO](https://github.com/apple/swift-nio?tab=readme-ov-file#swift-versions).

Otherwise, everything looks great! Thanks for contributing!!

@paulofaria
Copy link
Member

Nice work @3a4oT! 🙂

3a4oT added 2 commits October 18, 2024 15:26
- Adjusted CI to drop swift version less than 5.8
- run `swiftformat` locally
@3a4oT
Copy link
Contributor Author

3a4oT commented Oct 18, 2024

I'm okay merging this and dropping 5.8 after NIO does.

By the way, I think we should be explicit about our versioning support. The main question is do we phrase it as "the three latest released Swift versions" or do we phrase it as "we follow SwiftNIO support"? @NeedleInAJayStack

I agree. I think phrasing it as "we follow SwiftNIO support" makes sense at the moment while we're pretty reliant on NIO. If at some point we drop that dependency in favor of full Swift Concurrency we can reevaluate.

@3a4oT Could you add a Support section to the README between Encoding Results and Contributing that says:

This package supports Swift versions in [alignment with Swift NIO](https://github.com/apple/swift-nio?tab=readme-ov-file#swift-versions).

Otherwise, everything looks great! Thanks for contributing!!

Done

@NeedleInAJayStack NeedleInAJayStack merged commit 0dfa0d9 into GraphQLSwift:main Oct 21, 2024
6 checks passed
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.

3 participants