Skip to content

Apply package traits #490

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

Merged
merged 14 commits into from
Mar 6, 2025
Merged

Apply package traits #490

merged 14 commits into from
Mar 6, 2025

Conversation

fabianfett
Copy link
Member

Play around with package traits

@fabianfett fabianfett requested a review from sebsto March 1, 2025 13:08
@sebsto
Copy link
Contributor

sebsto commented Mar 2, 2025

Interesting change. Thank you.

Quick question. What is the command to compile and disable a specific trait ?
Unless I missed it, the trait pitch does not mention the change on the sift driver command line.

Copy link
Contributor

@FranzBusch FranzBusch left a comment

Choose a reason for hiding this comment

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

Love it!

Comment on lines +18 to +20
"FoundationJSONSupport",
"ServiceLifecycleSupport",
"LocalServerSupport",
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I don't know if this needs the support suffix but not feeling too strongly

@sebsto
Copy link
Contributor

sebsto commented Mar 5, 2025

Cam we wait for 6.1 to be GA before merging this ?

@sebsto
Copy link
Contributor

sebsto commented Mar 5, 2025

        // this enables all default traits
        .package(
            url: "https://github.com/swift-server/swift-aws-lambda-runtime.git",
            branch: "ff-package-traits"
        ),

        // for this reason the above is equivalent to this
        .package(
            url: "https://github.com/swift-server/swift-aws-lambda-runtime.git",
            branch: "ff-package-traits",
            traits: [.default]
        ),

        // for a user that doesn't want any trait they have to opt out
        .package(
            url: "https://github.com/swift-server/swift-aws-lambda-runtime.git",
            branch: "ff-package-traits",
            traits: []
        ),

        // just picking the traits that are wanted
        .package(
            url: "https://github.com/swift-server/swift-aws-lambda-runtime.git",
            branch: "ff-package-traits",
            traits: ["FoundationJSONSupport"]
        ),

// for testing only
.library(name: "AWSLambdaTesting", targets: ["AWSLambdaTesting"]),
],
traits: [
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this supported by the Swift 6.0 driver ?

Copy link
Member Author

Choose a reason for hiding this comment

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

oversight. hupps.

)
),
.testTarget(
name: "AWSLambdaRuntimeCoreTests",
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that AWSLambdaRuntimeCore doesn't exist, we should merge the tests too

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's do this in a follow up to keep the changes as small as possible.

Copy link
Contributor

@sebsto sebsto Mar 5, 2025

Choose a reason for hiding this comment

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

ok, let be sure we file an issue to track this otherwise it might stay like this forever :-)

@sebsto
Copy link
Contributor

sebsto commented Mar 5, 2025

@fabianfett can you merge with main Sources/AWSLambdaRuntimeCore/Lambda+LocalServer.swift ? This will solve the conflict and will allow to start the CI again

@fabianfett fabianfett marked this pull request as ready for review March 5, 2025 14:55
@fabianfett fabianfett added the ⚠️ semver/major Breaks existing public API. label Mar 5, 2025
Copy link
Contributor

@sebsto sebsto left a comment

Choose a reason for hiding this comment

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

LGTM. I'll create issue to track follow up work

@fabianfett fabianfett merged commit 0a75e0f into main Mar 6, 2025
28 of 31 checks passed
@fabianfett fabianfett deleted the ff-package-traits branch March 6, 2025 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚠️ semver/major Breaks existing public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants