-
Notifications
You must be signed in to change notification settings - Fork 116
refactor the Swift Settings in Package.swift
#558
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
Conversation
Package.swift
Outdated
@@ -2,6 +2,17 @@ | |||
|
|||
import PackageDescription | |||
|
|||
let defaultSwiftSettings: [SwiftSetting] = | |||
[ | |||
.swiftLanguageMode(.v6) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
swift language v6 is on by default if you select a 6.0 or later toolchain, so this isn't needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be worthwhile keeping this to enable some of the other optional features like ExistentialAny, MemberImportVisibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I started working on this because Fabian suggested to use
.enableExperimentalFeature(
"AvailabilityMacro=LambdaSwift 2.0:macOS 15.0"
)
on Swift 6.1
I will probably expand this PR scope
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adam-fowler Do you think it's OK to use the new @availability
macro in 6.1 and keep the old .platform[.macOS(.v15)]
in 6.0 ?
Package-6.0.swift :
platforms: [.macOS(.v15)],
Package.swift
.enableExperimentalFeature(
"AvailabilityMacro=LambdaSwift 2.0:macOS 15.0"
)
And in the code,
#if swift(>=6.1)
@available(LambdaSwift 2.0, *)
#endif
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
swift language v6 is on by default if you select a 6.0 or later toolchain, so this isn't needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
swift language v6 is on by default if you select a 6.0 or later toolchain, so this isn't needed
haha yeah I guess so. Leave it, remove it doesn't really matter. Maybe I missed something.
I think you can leave the platform requirement in. It's not like a project will include the swift-aws-lambda-runtime package and not use it. It is more likely to happen for client libraries (like valkey) where the library might be set to be available based on the existence of a trait.
@adam-fowler Fabian has a good point about the availability macro
Would that help to remove the |
Doesn't really affect HB. The only time we include this package as a dependency is when we need to use it, so the platform requirement is valid. |
@available
macro to remove requirement on.platform
in Package.swift.Package.swift
Motivation:
Package.swift
. This allows library builders and end users to be more flexible on their dependency requirements.Modifications:
var swiftSetting: [SwiftSettings]
and reuse it for all targets.AvailabilityMacro=LambdaSwift 2.0:macOS 15.0
@available(LambdaSwift 2.0, *)
Result:
There is no more SPM dependency on macOS 15