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: setup DynamoDbMapper publication #1419

Merged
merged 15 commits into from
Oct 3, 2024
Merged

Conversation

lauzadis
Copy link
Member

@lauzadis lauzadis commented Sep 26, 2024

This PR sets up DynamoDBMapper and other related libraries for publication. It also removes the dynamodb-mapper-annotation-processor-test module since it's now duplicated by SchemaGeneratorPluginTest

Issue #

Description of changes

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

Copy link

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK

@lauzadis lauzadis changed the title Feat ddb publish feat: setup DynamoDbMapper publication Oct 1, 2024
@lauzadis lauzadis marked this pull request as ready for review October 1, 2024 21:01
@lauzadis lauzadis requested a review from a team as a code owner October 1, 2024 21:01
Copy link

github-actions bot commented Oct 1, 2024

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK

Comment on lines 36 to 43
@OptIn(InternalApi::class)
fun getHllPreviewVersion() = if (sdkVersion.contains("-SNAPSHOT")) { // i.e. 1.3.29-beta-SNAPSHOT
sdkVersion
.removeSuffix("-SNAPSHOT")
.ensureSuffix("-beta-SNAPSHOT")
} else {
sdkVersion.ensureSuffix("-beta") // i.e. 1.3.29-beta
}
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.e." → "e.g."

Comment on lines 36 to 43
@OptIn(InternalApi::class)
fun getHllPreviewVersion() = if (sdkVersion.contains("-SNAPSHOT")) { // i.e. 1.3.29-beta-SNAPSHOT
sdkVersion
.removeSuffix("-SNAPSHOT")
.ensureSuffix("-beta-SNAPSHOT")
} else {
sdkVersion.ensureSuffix("-beta") // i.e. 1.3.29-beta
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: Any reason not to make this a val instead of a fun?

Comment on lines -40 to -43
val sdkVersion: String by project
group = "aws.sdk.kotlin"
version = sdkVersion

Copy link
Contributor

Choose a reason for hiding this comment

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

Question: What version will hll-codegen use now?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

OK so we'll just re-add version = sdkVersion when it comes time to GA the mapper?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes and it's set up so we just have to change it in that one spot

Comment on lines +1 to +2
import aws.smithy.kotlin.runtime.InternalApi
import aws.smithy.kotlin.runtime.text.ensureSuffix
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: How are these imports resolvable by the build script? Do we have the SDK/Smithy runtime in the classpath at this point?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure why it works like this, but we do have a dependency on smithy-kotlin:runtime-core in this module, and the imports to resolve

Comment on lines 45 to 54
subprojects {
group = "aws.sdk.kotlin"
version = getHllPreviewVersion()
configurePublishing("aws-sdk-kotlin")
}

subprojects {
if (!needsKmpConfigured) {
return@subprojects
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: Why was group, version, and configurePublishing refactored into a second subprojects block?

Copy link
Member Author

@lauzadis lauzadis Oct 2, 2024

Choose a reason for hiding this comment

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

All subprojects should have those values configured, they are not KMP-specific which is what the pre-existing block is for

Comment on lines 90 to 101
// FIXME Commonize the following functions into the aws-kotlin-repo-tools build-support
val sdkVersion: String by project

@OptIn(InternalApi::class)
fun getHllPreviewVersion() = if (sdkVersion.contains("-SNAPSHOT")) { // i.e. 1.3.29-beta-SNAPSHOT
sdkVersion
.removeSuffix("-SNAPSHOT")
.ensureSuffix("-beta-SNAPSHOT")
} else {
sdkVersion.ensureSuffix("-beta") // i.e. 1.3.29-beta
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Question: When commonizing this function I'm assuming we'll also commonize the other declaration of getHllPreviewVersion()?

Nit: If so can we add a // FIXME or a // TODO to the other instance to remind us

Copy link
Member Author

Choose a reason for hiding this comment

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

Personally I don't think this function will last long enough, it should be removed once we GA the DynamoDbMapper

Comment on lines +63 to +72
tasks.withType<PublishToMavenRepository> {
dependsOn(tasks.named("javadocJar"))
}

tasks.named("publishDynamodb-mapper-schema-generatorPluginMarkerMavenPublicationToMavenLocal") {
dependsOn(tasks.named("javadocJar"))
}

tasks.findByName("signDynamodb-mapper-schema-generatorPluginMarkerMavenPublication")
?.dependsOn(tasks.named("javadocJar"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Refactor tasks.named("javadocJar") into a val

Copy link

sonarcloud bot commented Oct 2, 2024

Copy link

github-actions bot commented Oct 2, 2024

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK

@lauzadis lauzadis merged commit 997a9f6 into feat-ddb-mapper Oct 3, 2024
11 checks passed
@lauzadis lauzadis deleted the feat-ddb-publish branch October 3, 2024 15:23
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.

3 participants