Skip to content

Conversation

@aajtodd
Copy link
Contributor

@aajtodd aajtodd commented Jan 25, 2024

Issue #

upstream aws/aws-kotlin-repo-tools#32

Description of changes

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions
Copy link

A new generated diff is ready to view.

@aajtodd aajtodd added the no-changelog Indicates that a changelog entry isn't required for a pull request. Use sparingly. label Jan 26, 2024
@aajtodd aajtodd marked this pull request as ready for review January 26, 2024 15:38
@aajtodd aajtodd requested a review from a team as a code owner January 26, 2024 15:38
build.gradle.kts Outdated
// into the root buildscript classpath and force them to share a class loader.
classpath(libs.smithy.model)
classpath(libs.smithy.aws.traits)
// // FIXME - we need the ClassLoader used for Model and any traits to be the same. Unfortunately our
Copy link
Contributor

@0marperez 0marperez Jan 26, 2024

Choose a reason for hiding this comment

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

nit: There's double comments here: // //

build.gradle.kts Outdated
// // that uses both in it's build logic won't work correctly. We "fix" this by placing them both
// // into the root buildscript classpath and force them to share a class loader.
// classpath(libs.smithy.model)
// classpath(libs.smithy.aws.traits)
Copy link
Contributor

Choose a reason for hiding this comment

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

question: Is this meant to be left here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no removing

plugins {
kotlin("jvm") // FIXME - configuration doesn't resolve without this
id("aws.sdk.kotlin.gradle.smithybuild")
kotlin("jvm")
Copy link
Contributor

Choose a reason for hiding this comment

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

question: Does the FIXME still apply?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed it because there isn't really anything to fix. It was a placeholder but everything works as intended so I don't see the value in keeping it if it's not actionable.

@sonarqubecloud
Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@github-actions
Copy link

A new generated diff is ready to view.

@aajtodd aajtodd merged commit 10d8f6d into main Feb 5, 2024
@aajtodd aajtodd deleted the refactor-plugin-apply branch February 5, 2024 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Indicates that a changelog entry isn't required for a pull request. Use sparingly.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants