-
Notifications
You must be signed in to change notification settings - Fork 79
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
feat: Generate package manifest for preview builds #1863
Conversation
@@ -35,14 +35,18 @@ struct GeneratePackageManifestCommand: ParsableCommand { | |||
@Flag(help: "If the package manifest should exclude runtime tests.") | |||
var excludeRuntimeTests = false | |||
|
|||
@Flag(help: "If the package manifest should be configured for a preview build.") | |||
var previewBuild = false | |||
|
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.
The CLI flag that is being added. Note it defaults to false, which renders a manifest with current behavior.
private func buildPreviewFlag() -> String { | ||
"let isPreviewBuild = \(previewBuild)" | ||
} | ||
|
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.
A simple boolean variable is rendered into the Package.swift
. It is true
when the CLI flag is set.
return .package(path: developmentPath) | ||
} else { | ||
return .package(url: gitURL, exact: clientRuntimeVersion) | ||
} |
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.
If isPreviewBuild
is set, a copy of smithy-swift
that is in a subfolder in the root of the SDK project will be used.
Otherwise, the smithy-swift
dependency will be set according to the current logic.
@@ -10,32 +10,72 @@ import XCTest | |||
|
|||
class PackageManifestBuilderTests: XCTestCase { | |||
|
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.
Below, manifest generator tests are added/modified for the new flag.
@@ -20,6 +20,8 @@ let crtVersion: Version = "0.43.0" | |||
|
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.
Regenerated manifest with changes is below.
@@ -31,6 +31,9 @@ codegen/protocol-test-codegen-local/smithy-build.json | |||
codegen/protocol-test-codegen/smithy-build.json | |||
SmokeTests/ | |||
|
|||
# Preview embedded dependencies | |||
/smithy-swift/ | |||
|
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.
We'll be embedding smithy-swift
in the SDK repo in preview build artifacts. This .gitignore
will make it so that if a preview customer wants to push their SDK up to Git, smithy-swift
won't get pushed with it.
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.
lgtm
…d-request.json file
@@ -25,7 +25,7 @@ struct FeaturesReader { | |||
} | |||
|
|||
struct Features: Decodable { | |||
let features: [Feature] | |||
let features: [Feature]? |
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.
Prevents build from failing when there are no features
Description of changes
The new
--preview-build
flag on thegenerate-package-manifest
command causes the SDK to use a copy ofsmithy-swift
that will be bundled inside the project specially in preview build artifacts.A follow-on change will utilize this flag in the build system.
New/existing dependencies impact assessment, if applicable
No new dependencies were added to this change.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.