-
Notifications
You must be signed in to change notification settings - Fork 4
refactor Ktor EAP trigger settings: simplify project and configuration handling, streamline logging messages #173
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
Conversation
…n handling, streamline logging messages
WalkthroughThis PR refactors a TeamCity Gradle init script to consolidate repository configuration into a global all-projects block, add mavenContent filtering for io.ktor in the EAP repository, and switch the resolution strategy from per-project to a global configurations.all approach with updated variable naming and logging statements. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.teamcity/src/subprojects/train/TriggerProjectSamplesOnEAP.kt (3)
96-106: Minor: trim and read KTOR_VERSION once per project for clarityFunctionally OK. Small cleanups reduce per-dependency env reads and avoid whitespace pitfalls.
- configurations.all { - resolutionStrategy { - eachDependency { - if (requested.group == "io.ktor") { - val ktorVersion = System.getenv("KTOR_VERSION") + val ktorVersion = System.getenv("KTOR_VERSION")?.trim() + configurations.all { + resolutionStrategy { + eachDependency { + if (requested.group == "io.ktor") { if (ktorVersion.isNullOrBlank()) { throw GradleException("KTOR_VERSION environment variable is not set or is blank. Cannot resolve Ktor EAP dependencies.") } - useVersion(ktorVersion) + useVersion(ktorVersion) logger.lifecycle("Forcing Ktor dependency " + requested.name + " to use EAP version: " + ktorVersion) } }
108-110: Avoid global 0s caches; they degrade performance and aren’t needed when versions are fixedYou already force concrete io.ktor versions via useVersion(ktorVersion). Disabling caches globally slows every resolve.
- cacheDynamicVersionsFor(0, "seconds") - cacheChangingModulesFor(0, "seconds")
113-121: Prefer a single root-level projectsEvaluated hook over per-project afterEvaluateThis avoids per-project callbacks and is order-safe for logging.
- afterEvaluate { - if (this == rootProject) { - logger.lifecycle("Project " + name + ": Using Ktor EAP version " + System.getenv("KTOR_VERSION")) - logger.lifecycle("Project " + name + ": EAP repository: https://maven.pkg.jetbrains.space/public/p/ktor/eap") - val pluginVersion = System.getenv("KTOR_GRADLE_PLUGIN_VERSION") - if (pluginVersion != null && pluginVersion.isNotEmpty()) { - logger.lifecycle("Project " + name + ": Using latest Ktor Gradle plugin version " + pluginVersion) - } - } - } + // Log once after all projects are evaluated + gradle.projectsEvaluated { + val root = rootProject + root.logger.lifecycle("Project " + root.name + ": Using Ktor EAP version " + System.getenv("KTOR_VERSION")) + root.logger.lifecycle("Project " + root.name + ": EAP repository: https://maven.pkg.jetbrains.space/public/p/ktor/eap") + System.getenv("KTOR_GRADLE_PLUGIN_VERSION")?.takeIf { it.isNotEmpty() }?.let { + root.logger.lifecycle("Project " + root.name + ": Using latest Ktor Gradle plugin version " + it) + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.teamcity/src/subprojects/train/TriggerProjectSamplesOnEAP.kt(2 hunks)
🔇 Additional comments (2)
.teamcity/src/subprojects/train/TriggerProjectSamplesOnEAP.kt (2)
85-95: Verify target projects support Gradle ≥6.8 before adopting exclusiveContentThe suggested refactor to use
exclusiveContentinstead ofmavenContentis stricter and better practice, but it requires Gradle ≥6.8. Since this TeamCity script generates configuration for downstream projects, you must confirm those projects' Gradle versions support this feature before applying the change.The proposed refactor remains:
- repositories { - maven { - name = "KtorEAP" - url = uri("https://maven.pkg.jetbrains.space/public/p/ktor/eap") - mavenContent { - includeGroup("io.ktor") - } - } - } + repositories { + exclusiveContent { + forRepository { + maven { + name = "KtorEAP" + url = uri("https://maven.pkg.jetbrains.space/public/p/ktor/eap") + } + } + filter { + includeGroup("io.ktor") + } + } + }Verify the Gradle wrapper versions in projects consuming this configuration before merging.
31-49: Verify Plugin Portal API ordering to confirm if .versions[0] is reliably the latestNo official
/versions/latestendpoint exists on Gradle Plugin Portal, so querying the full API is the correct approach. However, the current implementation assumes.versions[0]is the latest version without confirming the Plugin Portal API guarantees this ordering.The reviewer's suggestion to implement semver sorting is defensible—if the Plugin Portal returns versions in an unspecified order, fetching the first element could yield incorrect results and cause regressions. Before applying the suggested fixes, confirm with the Plugin Portal documentation or test the actual API response to verify that versions are consistently sorted by release date (newest first).
No description provided.