Skip to content

Conversation

lauzadis
Copy link
Member

@lauzadis lauzadis commented Sep 29, 2023

This PR adds a basic, empty implementation of a new KMP target, linuxX64. This is required to unblock our upgrade to Dokka 1.9.0. It also serves as a proof-of-concept of adding a new target to the SDK.

Issue #

Related to but does not fully complete aws-sdk-kotlin#229

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
Contributor

Choose a reason for hiding this comment

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

If possible we should strive to keep these tests running for all platforms, refactoring if necessary.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm able to keep this in common now that I removed SdkIOException

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, some of the tests use mockk which has no multiplatform support, so they must stay in JVM. I split the testcase across two platforms for now


// FIXME KMP implementation
@Suppress("ACTUAL_WITHOUT_EXPECT")
internal actual class SdkIOException : Exception()
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we even need this anymore? We have IOException in runtime-core...

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, I removed it, thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

What was platform specific about these?

Copy link
Member Author

Choose a reason for hiding this comment

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

The benchmark application definition in build.gradle.kts. I got many unresolved symbol errors when running :tests:benchmarks:service-benchmarks:compileKotlinLinuxX64. We have this segment of code in the build file which seems JVM-specific.

tasks.named<JavaExec>("run") {
    classpath += objects.fileCollection().from(
        tasks.named("compileKotlinJvm"),
        configurations.named("jvmRuntimeClasspath"),
    )
}

@github-actions
Copy link

A new generated diff is ready to view.

@lauzadis lauzadis changed the title feat: add a dummy implementation of a second KMP target, LinuxX64 feat: add base implementation of linuxX64 target Oct 13, 2023
@github-actions
Copy link

A new generated diff is ready to view.

@github-actions
Copy link

A new generated diff is ready to view.

@lauzadis lauzadis marked this pull request as ready for review October 16, 2023 18:13
@lauzadis lauzadis requested a review from a team as a code owner October 16, 2023 18:13
@github-actions
Copy link

A new generated diff is ready to view.

@github-actions
Copy link

A new generated diff is ready to view.

@github-actions
Copy link

A new generated diff is ready to view.

@github-actions
Copy link

A new generated diff is ready to view.

@github-actions
Copy link

A new generated diff is ready to view.

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

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

No Coverage information No Coverage information
7.8% 7.8% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

secretAccessKey = "secret"
}
httpClient = NoHttpEngine
}.use { polly ->
Copy link
Contributor

Choose a reason for hiding this comment

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

You just need to import our use extension

@lauzadis lauzadis merged commit e9a321e into main Oct 25, 2023
@lauzadis lauzadis deleted the feat-kmp-targets branch November 22, 2023 21:19
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