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

refactor: split AWS SDK specific customizations into new module #1151

Merged
merged 15 commits into from
Dec 15, 2023

Conversation

aajtodd
Copy link
Collaborator

@aajtodd aajtodd commented Dec 12, 2023

Issue #

upstream: smithy-lang/smithy-kotlin#1012

Description of changes

This PR is a first step to separating AWS protocol support from AWS SDK specific customizations. Specifically we introduce a new module aws-sdk-codegen which is everything AWS SDK for Kotlin specific. The existing smithy-aws-kotlin-codegen now only contains AWS protocol support.

refactor: Split out AWS SDK for Kotlin specific customizations into a new module aws-sdk-codegen
refactor: Use the software.amazon.smithy.kotlin.codegen.aws namespace for AWS protocol support instead of aws.sdk.kotlin.codegen to better align with the rest of the code generation in smithy-kotlin
refactor: Remove S3Generator in favor of overriding sections
fix: Fix route53 customization to only apply to the operation it is meant to
refactor: Migrate flow utils to smithy-kotlin runtime.

Follow up work

The line between "AWS SDK" and "AWS Protocol" isn't super well defined. We may find I got some of this wrong and need to move things around a bit more 🤷‍♂️.

  • Migrate AWS protocol support to smithy-kotlin
  • Migrate protocol tests to smithy-kotlin

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@aajtodd aajtodd requested a review from a team as a code owner December 12, 2023 20:17
Copy link

A new generated diff is ready to view.

Copy link
Member

@lauzadis lauzadis left a comment

Choose a reason for hiding this comment

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

Looks like a good first step! I had one question about the removed Maven publication configuration

Comment on lines -114 to -123
publishing {
publications {
create<MavenPublication>("codegen") {
from(components["java"])
artifact(sourcesJar)
}
}
}

configurePublishing("aws-sdk-kotlin")
Copy link
Member

Choose a reason for hiding this comment

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

question: does this no longer need to be published?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We never should have published it in first place, we will publish it when we move it over to smithy-kotlin though.

Copy link

A new generated diff is ready to view.

@aajtodd aajtodd added the no-changelog Indicates that a changelog entry isn't required for a pull request. Use sparingly. label Dec 14, 2023
Copy link

sonarcloud bot commented Dec 14, 2023

Quality Gate Failed Quality Gate failed

Failed conditions

5.4% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@aajtodd aajtodd merged commit 1e99683 into main Dec 15, 2023
13 of 16 checks passed
@aajtodd aajtodd deleted the refactor-aws-protocols branch December 15, 2023 01:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Indicates that a changelog entry isn't required for a pull request. Use sparingly.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants