chore: extract bootstrap into separate project with tests #1172
Conversation
aajtodd
added
the
no-changelog
|
A new generated diff is ready to view. |
| }, | ||
| "packageHandlingRules": { | ||
| "versioning": { | ||
| "defaultVersionLayout": "{MAJOR}.0.x" |
There was a problem hiding this comment.
note: We will have to remember to change this for our next minor version bump. There's a tradeoff between security (not accidentally bumping version) and maintainability (remembering this also needs to change).
Maybe this should be documented somewhere (e.g internal runbook) for bumping versions?
There was a problem hiding this comment.
note: We will have to remember to change this for our next minor version bump.
If we land where I hope we land (which is to have only one internal major version) then I expect this won't ever change even when we bump minor version externally. This is set for now to preserve the existing major version and not accidentally bump it (introducing a new MV internally).
build-support/src/main/kotlin/aws/sdk/kotlin/gradle/sdk/Naming.kt
Outdated
Show resolved
Hide resolved
build-support/src/main/kotlin/aws/sdk/kotlin/gradle/sdk/AwsService.kt
Outdated
Show resolved
Hide resolved
| project.logger.info("discovered service: ${serviceTrait.sdkId}") | ||
| AwsService( | ||
| serviceShapeId = service.id.toString(), | ||
| packageName = packageNamespaceForService(sdkId), |
There was a problem hiding this comment.
correctness: should this be packageNameForService(sdkId)?
it's correct, but I had a comment about the naming below
| /** | ||
| * Get the package namespace for a service from it's `sdkId` | ||
| */ | ||
| fun packageNamespaceForService(sdkId: String): String = "$SDK_PACKAGE_NAME_PREFIX${packageNameForService(sdkId)}" |
There was a problem hiding this comment.
naming: namespacedPackageNameForService?
while reading this, I was expecting packageNamespace to just return $SDK_PACKAGE_NAME_PREFIX, not the package name as well
| fun Membership.isMember(vararg memberNames: String): Boolean = | ||
| memberNames.none(exclusions::contains) && (inclusions.isEmpty() || memberNames.any(inclusions::contains)) | ||
| fun parseMembership(rawList: String?): Membership { | ||
| if (rawList == null) return Membership() |
There was a problem hiding this comment.
nit: this early return seems not necessary
There was a problem hiding this comment.
Much of this was ported as is
| fun packageNameForService(sdkId: String): String = | ||
| sdkId.replace(" ", "") | ||
| .replace("-", "") | ||
| .lowercase() | ||
| .kotlinNamespace() |
There was a problem hiding this comment.
simplification: fun packageNameForService(sdkId: String): String = sdkIdToArtifactName(sdkId).kotlinNamespace()
There was a problem hiding this comment.
I didn't want to change anything from what it was (yet anyway) so much of the naming is ported exactly as it was.
| listOf( | ||
| RestJson1Trait.ID, | ||
| RestXmlTrait.ID, | ||
| AwsJson1_0Trait.ID, | ||
| AwsJson1_1Trait.ID, | ||
| AwsQueryTrait.ID, | ||
| Ec2QueryTrait.ID, | ||
| ).first { hasTrait(it) }.name |
There was a problem hiding this comment.
comment: I feel like Smithy should provide this list of protocol traits
they don't (I checked) but it seems like this defining this custom list will break something in the future
There was a problem hiding this comment.
Yeah this is unfortunate but luckily the list of protocols doesn't change much over time.
build-support/src/test/kotlin/aws/sdk/kotlin/gradle/sdk/MembershipTest.kt
Show resolved
Hide resolved
|
A new generated diff is ready to view. |
aajtodd
force-pushed
the
publishing-qa
branch
from
a622d54 to
7398f53
Compare
|
A new generated diff is ready to view. |
|
A new generated diff is ready to view. |
|
A new generated diff is ready to view. |
aajtodd
force-pushed
the
publishing-qa
branch
from
f4ce9f6 to
a11fe0d
Compare
|
|
A new generated diff is ready to view. |
Issue #
n/a
Description of changes
refactor: Add a new
.brazil.jsonfile with the transform logic for justaws-sdk-kotlin. NOTE: This will not take effect until we release a new version ofkatand update the release pipeline to use it. Until then the "global" transform config from repo tools is still the one being used. I tested this locally already on 5 services (all protocols) + the runtime and diffed the transform output using the one from repo tools and this new localized copyrefactor: Extract a lot of the logic used to bootstrap the AWS services codegen tasks into a separate composite build. This allows us to actually add tests for our logic and have higher confidence in changes to the build. There is more to do here but I think this is a decent point to stop and review before continuing with any refactoring or adding additional tests.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.