Skip to content

Conversation

@ianbotsf
Copy link
Contributor

Issue #

Closes #460

Switches to KMP codegen for service clients.

Companion PR: smithy-kotlin#646

Description of changes

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

@github-actions
Copy link

A new generated diff is ready to view: __generated-main...__generated-feat-kmp-codegen

Copy link
Contributor

@aajtodd aajtodd left a comment

Choose a reason for hiding this comment

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

Looks great. Fix and ship.

NOTE: This should be considered a breaking change and we probably want a more detailed description in the changelog about it. The reason being that the jvm artifact is now -jvm (e.g. s3-jvm) and not the existing artifact (e.g. s3) which is now common.

If you are not using gradle (or the kotlin plugin) you need to pull in the -jvm artifact now. This should be handled automatically for kotlin gradle plugin users.

@@ -1,14 +1,19 @@
import org.jetbrains.kotlin.gradle.targets.jvm.tasks.KotlinJvmTest
Copy link
Contributor

Choose a reason for hiding this comment

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

fix: license header should come first

// fixes outgoing variant metadata: https://github.com/awslabs/smithy-kotlin/issues/258
fun Project.setOutgoingVariantMetadata() {
tasks.withType<JavaCompile>() {
val javaVersion = JavaVersion.VERSION_1_8.toString()
Copy link
Contributor

Choose a reason for hiding this comment

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

this probably needs an equivalent somewhere in the kotlin compilation options? It looks like we used to set it on line 78.


if (testBucket == null) {
testBucket = prefix + UUID.randomUUID()
println("Creating S3 bucket: $testBucket")
Copy link
Contributor

Choose a reason for hiding this comment

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

probably be good to add an else branch and print that we are re-using an existing test bucket (which could happen if delete fails or tests are ended abruptly, etc).

@github-actions
Copy link

A new generated diff is ready to view: __generated-main...__generated-feat-kmp-codegen

@github-actions
Copy link

A new generated diff is ready to view: __generated-main...__generated-feat-kmp-codegen

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@github-actions
Copy link

A new generated diff is ready to view: __generated-main...__generated-feat-kmp-codegen

@ianbotsf ianbotsf merged commit 57346af into main May 24, 2022
@ianbotsf ianbotsf deleted the feat-kmp-codegen branch May 24, 2022 15:10
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.

Use of library from commonMain in Kotlin Multiplatform Project

2 participants