update Ktor EAP trigger settings: switch to dependencyResolutionManagement for repository configuration, refine structure and enhance consistency with Gradle settings #221
Conversation
…ement for repository configuration, refine structure and enhance consistency with Gradle settings
WalkthroughMigrates Gradle repository configuration from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.teamcity/src/subprojects/train/TriggerProjectSamplesOnEAP.kt (3)
24-86: Reduce duplication of repository templates inEapRepositoryConfigand callersThe new
EapRepositoryConfighelpers nicely centralize URLs and the maven repositories, but the NodeJS/Yarn ivy definitions are still hand‑written here and re‑encoded in the init script and root EAP override. Consider extracting an additional helper (e.g.,generateIvyRepositories()or a fullgenerateSettingsRepositories()) and reusing it in all places that emit settings/dependencyResolutionManagementblocks to avoid future drift between these definitions.
138-185: Align init‑script repositories with shared settings helpers and unify NodeJS repo nameThe
gradle.settingsEvaluated { settings -> settings.dependencyResolutionManagement { ... } }block looks correct and matches the intended repo set, but it now duplicates essentially the same configuration thatEapRepositoryConfig.generateSettingsContent()and the root EAP override emit. To minimize divergence, consider driving this block from the same helpers (at least for the repository body). Also, the NodeJS ivy repo is named"Node.js"here while it is"NodeJS"elsewhere; renaming it for consistency will make logs and diagnostics easier to correlate.Example for the naming tweak:
- ivy { - name = "Node.js" + ivy { + name = "NodeJS"
377-451: Consider reusinggenerateSettingsContent()in root EAP override and watch for duplicatedependencyResolutionManagementblocksThe
eap-settings.gradle.ktscontent inmodifyRootSettingsForEAPis almost identical toEapRepositoryConfig.generateSettingsContent(), which means you now have two parallel copies of the samedependencyResolutionManagementblock. To avoid divergence, you can generate the root override from the same helper you already use increateEAPSampleSettings:- cat > "${'$'}EAP_SETTINGS_FILE" << 'EOF' -dependencyResolutionManagement { - repositoriesMode.set(RepositoriesMode.PREFER_SETTINGS) - repositories { - mavenCentral() - google() - maven("${EapRepositoryConfig.COMPOSE_DEV_URL}") { - content { - excludeGroup("org.nodejs") - } - } - maven { - name = "KtorEAP" - url = uri("${EapRepositoryConfig.KTOR_EAP_URL}") - content { - includeGroup("io.ktor") - } - } - ivy { - name = "NodeJS" - url = uri("https://nodejs.org/dist") - patternLayout { - artifact("v[revision]/[artifact](-v[revision]-[classifier]).[ext]") - } - metadataSources { - artifact() - } - content { - includeModule("org.nodejs", "node") - } - } - ivy { - name = "Yarn" - url = uri("https://github.com/yarnpkg/yarn/releases/download") - patternLayout { - artifact("v[revision]/[artifact]-v[revision].tar.gz") - } - metadataSources { - artifact() - } - content { - includeModule("com.yarnpkg", "yarn") - } - } - } -} -EOF + cat > "${'$'}EAP_SETTINGS_FILE" << 'EOF' +${EapRepositoryConfig.generateSettingsContent()} +EOFAlso, since this override introduces its own
dependencyResolutionManagement { repositoriesMode.set(RepositoriesMode.PREFER_SETTINGS) ... }, please verify behaviour on any projects whose existingsettings.gradle.ktsalready declare adependencyResolutionManagementblock to ensure Gradle is happy with both blocks being applied.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.teamcity/src/subprojects/train/TriggerProjectSamplesOnEAP.kt(1 hunks)
🔇 Additional comments (3)
.teamcity/src/subprojects/train/TriggerProjectSamplesOnEAP.kt (3)
18-22: Verify new BOM‑scoped Ktor EAP metadata URL
KTOR_EAP_METADATA_URLnow points to the BOM artifact metadata (.../io/ktor/ktor-bom/maven-metadata.xml) and is used in the “Fetch Latest EAP Framework Version” step. That’s a reasonable scope change, but if this endpoint is unavailable or lacks<latest>/matching-eap-versions, the resolver build will start failing at runtime. Please confirm this URL works from your agents (e.g., rerun the resolver build or a quickcurlcheck) and returns the expected EAP versions.Also applies to: 664-678
186-252: EAP version forcing viaallprojectsremains consistent with new settings configurationThe
allprojectsblock still forcesio.ktordependencies to theKTOR_VERSIONenvironment value and logs the chosen version, while the new settings‑time repository configuration only changes where dependencies are resolved from. This separation of concerns (repos in settings, version override inallprojects) looks coherent and should preserve existing behaviour.
304-321: Good reuse ofEapRepositoryConfig.generateSettingsContent()for sample settingsHaving
createEAPSampleSettingswritesettings.gradle.ktsviaEapRepositoryConfig.generateSettingsContent()centralizes the fulldependencyResolutionManagementdefinition for samples. That should make future repo tweaks much safer, since there’s now a single canonical source for the sample settings template.
No description provided.