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

feat: Merge AWS service model files at codegen time #1545

Merged
merged 4 commits into from
Jun 5, 2024

Conversation

jbelkins
Copy link
Contributor

@jbelkins jbelkins commented Jun 5, 2024

Issue #

#1543

Description of changes

smithy-lang/smithy-swift#752 adds an option to merge models at code-generation time.

Specific changes in SDK:

  • Model merge turned on by default for AWS service generation. All other code generation (protocol tests, etc.) continues to generate individual model files.
  • The mergeModels.sh script is no longer used and is removed. References to that script are removed throughout scripting & CI.

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.

@jbelkins jbelkins requested review from dayaffe and sichanyoo June 5, 2024 21:36
@jbelkins jbelkins marked this pull request as ready for review June 5, 2024 21:36
@@ -63,7 +63,6 @@ jobs:
./gradlew -p codegen/sdk-codegen build
./gradlew -p codegen/sdk-codegen stageSdks
./gradlew --stop
./scripts/mergeModels.sh Sources/Services
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Invocations of mergeModels.sh are removed here and in other scripts where it is currently used.

@@ -65,7 +65,8 @@ fun generateSmithyBuild(tests: List<CodegenTest>): String {
"swiftVersion": "5.7.0",
"build": {
"rootProject": true
}
},
"mergeModels": false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Protocol tests (either local or Smithy) do not merge models

@@ -117,7 +117,8 @@ fun generateSmithyBuild(services: List<AwsService>): String {
"build": {
"rootProject": $buildStandaloneSdk
},
"useInterceptors": ${interceptorsServices.contains(service.packageName)}
"useInterceptors": ${interceptorsServices.contains(service.packageName)},
"mergeModels": true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When generating an AWS SDK, models are merged. This can be changed to false for troubleshooting, if desired.

@@ -11,9 +11,6 @@ rm -rf Tests/Services/*
./gradlew -p codegen/sdk-codegen stageSdks
./gradlew --stop

# Merge model files
./scripts/mergeModels.sh Sources/Services

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The rest of the changes are the removal of the mergeModels.sh script.

@@ -25,6 +25,7 @@ import software.amazon.smithy.swift.codegen.model.toUpperCamelCase
import software.amazon.smithy.swift.codegen.swiftmodules.ClientRuntimeTypes.Middleware.NoopHandler
import software.amazon.smithy.swift.codegen.swiftmodules.SmithyHTTPAPITypes
import software.amazon.smithy.swift.codegen.swiftmodules.SmithyTypes
import software.amazon.smithy.swift.codegen.utils.ModelFileUtils
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The next several changes are just using the helper method to determine the filename for a generated model Swift file.

@jbelkins jbelkins merged commit f688b95 into main Jun 5, 2024
29 checks passed
@jbelkins jbelkins deleted the jbe/codegen_merge_models branch June 5, 2024 23:16
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.

2 participants