-
Notifications
You must be signed in to change notification settings - Fork 49
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: add base implementation of linuxX64 target #1058
Conversation
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
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
...
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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"),
)
}
A new generated diff is ready to view. |
A new generated diff is ready to view. |
A new generated diff is ready to view. |
…into feat-kmp-targets
A new generated diff is ready to view. |
A new generated diff is ready to view. |
A new generated diff is ready to view. |
A new generated diff is ready to view. |
A new generated diff is ready to view. |
SonarCloud Quality Gate failed. 0 Bugs No Coverage information Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
region = "us-east-2" | ||
credentialsProvider = StaticCredentialsProvider { | ||
accessKeyId = "AKID" | ||
secretAccessKey = "secret" | ||
} | ||
httpClient = NoHttpEngine | ||
}.use { polly -> |
There was a problem hiding this comment.
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
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.