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

Smithy4s #67

Merged
merged 11 commits into from
Sep 25, 2024
Merged

Smithy4s #67

merged 11 commits into from
Sep 25, 2024

Conversation

bpholt
Copy link
Member

@bpholt bpholt commented Nov 16, 2023

I think this demonstrates how to include a smithy4s reference in a library while being able to maintain binary compatibility.

} yield tagSecurableString(bytes)
case s => tagSecurableString(s).pure[F]
}
def SecureReader[F[_] : Async : Compression](awsEnv: AwsEnvironment[F]): Resource[F, ConfigReader[F[SecurableString]]] =
Copy link
Member Author

@bpholt bpholt Sep 23, 2024

Choose a reason for hiding this comment

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

The Smithy4s documentation recommends against "treating Smithy4s-generated code as publishable library-material." However, this interface doesn't expose the generated KMS classes in any way; rather it accepts an AwsEnvironment[F] (something published by smithy4s) and returns a ConfigReader[F[SecurableString]], where SecurableString is defined by this library. As such I think for MiMa purposes we can treat the code in com.amazonaws.kms as private to this library, meaning we shouldn't need to worry about its binary compatibility.

(It would be better to move com.amazonaws.kms to another package, e.g. com.dwolla.aws.kms or something like that, to minimize the chances of a clash with another library or generated code using com.amazonaws.kms. See the TODO in build.sbt. this has been addressed)

@bpholt bpholt force-pushed the smithy4s branch 2 times, most recently from 685585b to 6d4df7d Compare September 23, 2024 22:17
@bpholt bpholt marked this pull request as ready for review September 23, 2024 22:20
@bpholt bpholt requested a review from a team as a code owner September 23, 2024 22:20
@bpholt bpholt force-pushed the smithy4s branch 3 times, most recently from cd47bde to 95f1fae Compare September 24, 2024 23:21
ThisBuild / startYear := Option(2018)
ThisBuild / tlBaseVersion := "0.4"
ThisBuild / tlJdkRelease := Some(8)
ThisBuild / githubWorkflowBuild := List(Sbt(List("compile", "test")))
Copy link
Member Author

Choose a reason for hiding this comment

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

Adding the extra compile step here ensures that sbt recompiles the code after it's modified by Scalafix. When the test task is executed after the explicit compile step, sbt will see that the code has changed since the compile step and recompile it.

|""".stripMargin)
}

test("we don't have access to the generated KMS objects in the shaded package") {
Copy link
Member Author

Choose a reason for hiding this comment

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

If this test is attempted without the extra compile step (i.e. sbt clean ++2.13 test), it will fail. If the code as-modified-by-Scalafix is compiled, the test should succeed.

The test is broken up into Scala 2 and Scala 3 variants because the compiler messages are different coming out of the different compilers. This is probably a little brittle but it should work for now.

@bpholt bpholt merged commit 6e962f0 into main Sep 25, 2024
12 checks passed
@bpholt bpholt deleted the smithy4s branch September 25, 2024 16:58
@mergify mergify bot added the rules label Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants