fix: correctly default compositeProjects build setting #1107
Conversation
ianbotsf
added
the
no-changelog
|
Kudos, SonarCloud Quality Gate passed!
|
| ?.map { file(it) } // Create file from path | ||
| ?.toList() | ||
| ?: emptyList() | ||
| val propertyVal = localProperties.getProperty("compositeProjects") ?: "../smithy-kotlin" |
There was a problem hiding this comment.
Would it be better to throw an exception if there is no compositeProjects property (such as with checkNotNull { ... }) so it can fallback to the catch (e: Throwable) logic on L96? It would remove the (small) duplication
There was a problem hiding this comment.
local.properties is heavily used for Android development so it's entirely reasonable that the file may not have this key.
There was a problem hiding this comment.
if it's missing, that's fine, it will still default to ../smithy-kotlin, just in a different way
There was a problem hiding this comment.
Whoops, shipped as soon as I saw approvals but before I saw comments. Yes, it would've probably been better to clean up the duplication.
|
A new generated diff is ready to view.
|
Issue #
(none)
Description of changes
Our settings.gradle.kts build logic for composite projects doesn't correctly handle the situation where a local.properties file exists but does not contain a
compositeProjectskey. This change now handles that case by defaulting to ../smithy-kotlin.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.