-
Notifications
You must be signed in to change notification settings - Fork 50
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
misc: track upstream versioning changes #1106
Conversation
4fe057c
to
9904584
Compare
9904584
to
87bfd0c
Compare
A new generated diff is ready to view.
|
gradle/libs.versions.toml
Outdated
smithy-kotlin-version = "0.28.1" | ||
smithy-kotlin-codegen-version = "0.28.1" |
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.
Nit: I think we should go for smithy-kotlin-runtime-version
here to be clear.
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.
Ok, that'll be a broader change (including internal package) but I think it's best for clarity
@@ -28,7 +28,7 @@ def run(self, env): | |||
|
|||
print("discovered dependency versions: {}".format(discovered_versions)) | |||
if "smithy-kotlin" in discovered_versions: | |||
_replace_version_catalog_version(proj, "smithy-kotlin-version", discovered_versions["smithy-kotlin"]) | |||
_replace_version_catalog_version(proj, "smithy-kotlin-runtime-version", discovered_versions["smithy-kotlin"]) |
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.
We can probably remove this and builder.json
since we don't have a dependency on CRT builder (to my knowledge) anymore.
gradle/libs.versions.toml
Outdated
# smithy-kotlin codegen and runtime are versioned together | ||
smithy-kotlin-version = "0.28.1" | ||
# smithy-kotlin codegen and runtime are versioned separately | ||
smithy-kotlin-runtime-version = "0.28.1" |
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.
fix: There are various CI jobs that will need updated to handle this change.
A not necessarily complete survey:
- https://github.com/awslabs/smithy-kotlin/blob/main/.github/workflows/continuous-integration.yml#L110
- API ref docs construct in SDK release stack
- ??
We'll have to update both versions now to match where we do this.
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.
Yeah, I made all those changes already (global code search smithy-kotlin-version
across our internal and public repositories), I just forgot to push 🤦
A new generated diff is ready to view.
|
A new generated diff is ready to view.
|
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
A new generated diff is ready to view.
|
Track upstream smithy-kotlin changes in which the runtime and codegen versioning had been split. smithy-lang/smithy-kotlin#992
Issue #
Description of changes
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.