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

chore: upgrade smithy to 1.13 #520

Merged
merged 4 commits into from
Nov 2, 2021
Merged

chore: upgrade smithy to 1.13 #520

merged 4 commits into from
Nov 2, 2021

Conversation

aajtodd
Copy link
Contributor

@aajtodd aajtodd commented Nov 1, 2021

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.

@aajtodd aajtodd requested review from kggilmer and ianbotsf and removed request for kggilmer November 1, 2021 15:39
Copy link
Contributor

@kggilmer kggilmer left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -42,6 +42,8 @@ val generateSdkRuntimeVersion by tasks.registering {
// this keeps us from having to manually change version numbers in multiple places
val resourcesDir = "$buildDir/resources/main/software/amazon/smithy/kotlin/codegen/core"
val versionFile = file("$resourcesDir/sdk-version.txt")
val gradlePropertiesFile = rootProject.file("gradle.properties")
inputs.file(gradlePropertiesFile)
Copy link
Contributor

Choose a reason for hiding this comment

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

question

This is to determine build dirty-ness or something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. When we upgrade the version the generated version is not changing because it is already generated. This generally seems to only affect local development because CI is a fresh build. The fix is to clean and rebuild (or manually delete the file).

Copy link
Contributor

@ianbotsf ianbotsf left a comment

Choose a reason for hiding this comment

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

Minor note about a file that doesn't belong in this commit. Otherwise, looks good.

Comment on lines 10 to 14
## Scope
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)

Copy link
Contributor

Choose a reason for hiding this comment

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

Correctness: Unrelated to upgrading Smithy. This should be moved to a separate commit.

@aajtodd aajtodd force-pushed the chore-upgrade-smithy branch from 3b40438 to 71e10ea Compare November 2, 2021 14:13
@sonarcloud
Copy link

sonarcloud bot commented Nov 2, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

No Coverage information No Coverage information
0.0% 0.0% Duplication

@aajtodd aajtodd merged commit 377c017 into main Nov 2, 2021
@aajtodd aajtodd deleted the chore-upgrade-smithy branch November 2, 2021 16:13
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