Vgoncharova/ktor train #158
Conversation
…vironment preparations
WalkthroughIntroduces an EAP sample config interface and refactors sample build orchestration into new BuildSteps helpers, centralizes Ktor EAP version resolution and propagation via an EAP version resolver (env.KTOR_VERSION), adds Gradle/Maven EAP build steps, and rewires composites and dependencies with OS-name agent checks. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.teamcity/src/subprojects/train/TriggerProjectSamplesOnEAP.kt(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
.teamcity/src/subprojects/train/TriggerProjectSamplesOnEAP.kt (1)
.teamcity/src/subprojects/build/ProjectBuild.kt (1)
defaultBuildFeatures(128-147)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.teamcity/src/subprojects/train/TriggerProjectSamplesOnEAP.kt (1)
256-297: Version resolution script is functional with minor improvement opportunity.The bash script correctly fetches and parses Maven metadata with appropriate error handling. The
grep/sedapproach works but could be fragile if XML formatting changes.For more robust XML parsing, consider using
xmllint:- # Extract the latest version using grep and sed - # This pattern looks for a <latest>version</latest> tag - LATEST_VERSION=$(grep -o '<latest>[^<]*</latest>' "${'$'}TEMP_FILE" | sed 's/<latest>\(.*\)<\/latest>/\1/') + # Extract the latest version using xmllint for robust XML parsing + LATEST_VERSION=$(xmllint --xpath 'string(//latest)' "${'$'}TEMP_FILE" 2>/dev/null)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.teamcity/src/subprojects/train/TriggerProjectSamplesOnEAP.kt(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
.teamcity/src/subprojects/train/TriggerProjectSamplesOnEAP.kt (1)
.teamcity/src/subprojects/build/ProjectBuild.kt (1)
defaultBuildFeatures(128-147)
🔇 Additional comments (6)
.teamcity/src/subprojects/train/TriggerProjectSamplesOnEAP.kt (6)
18-21: LGTM: Clean abstraction.The interface provides a clear contract for EAP sample configurations.
23-54: LGTM: Gradle init script is well-structured.The script correctly sets up the EAP repository and uses
System.getenv("KTOR_VERSION")for dependency resolution. The KTOR_VERSION environment variable is properly propagated in the build configurations at lines 148 and 188.
56-87: LGTM: Build logic correctly handles project variations.The function properly handles standalone vs. nested projects and distinguishes between plugin and regular samples.
89-128: LGTM: Maven configuration is correct.The Maven settings properly configure the EAP repository and use the resolved KTOR_VERSION. The propagation is correctly handled at line 188.
130-166: LGTM: KTOR_VERSION propagation correctly implemented.Both extension functions now properly propagate
env.KTOR_VERSIONfrom the versionResolver (lines 148 and 188), addressing the critical issues raised in previous reviews. The snapshot dependencies ensure the resolver runs first.Also applies to: 168-225
334-429: LGTM: Composite build structure is well-organized.The hierarchical dependency structure correctly chains the individual sample builds through composites to the top-level validation build, with appropriate triggers on EAP publications.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.teamcity/src/subprojects/train/TriggerProjectSamplesOnEAP.kt (3)
66-75: Consider clarifying the working directory logic.The combination of
workingDirand conditionalcdcommands works correctly but may be confusing for future maintainers. The logic is:
- Regular samples: set
workingDir = projectName- Plugin samples (standalone): work from root
- Plugin samples (non-standalone): work from root, then
cd samples/$projectNameConsider adding a comment explaining this structure or consolidating the logic.
109-109: Maven version property set both in settings and command line.The Ktor version is configured both as a profile property (Line 109) and as a command-line argument (Line 127). This ensures coverage regardless of how POMs reference the version, though it may be redundant if POMs consistently use one approach.
Also applies to: 127-127
277-279: Consider more robust XML parsing.The
grep | sedapproach for extracting the<latest>tag is fragile and may break if the XML contains unexpected whitespace or formatting. Consider usingxmllintfor more reliable parsing:- # Extract the latest version using grep and sed - # This pattern looks for a <latest>version</latest> tag - LATEST_VERSION=$(grep -o '<latest>[^<]*</latest>' "${'$'}TEMP_FILE" | sed 's/<latest>\(.*\)<\/latest>/\1/') + # Extract the latest version using xmllint + LATEST_VERSION=$(xmllint --xpath 'string(//latest)' "${'$'}TEMP_FILE" 2>/dev/null)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.teamcity/src/subprojects/train/TriggerProjectSamplesOnEAP.kt(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
.teamcity/src/subprojects/train/TriggerProjectSamplesOnEAP.kt (1)
.teamcity/src/subprojects/build/ProjectBuild.kt (1)
defaultBuildFeatures(128-147)
🔇 Additional comments (6)
.teamcity/src/subprojects/train/TriggerProjectSamplesOnEAP.kt (6)
19-22: LGTM: Clean abstraction for sample configurations.The interface design enables polymorphic handling of both plugin and regular samples, setting up a solid foundation for the EAP validation workflow.
24-55: LGTM: Init script correctly forces EAP versions.The Gradle init script properly overrides all
io.ktordependencies viaresolutionStrategy.eachDependency. TheKTOR_VERSIONenvironment variable is correctly propagated from the version resolver (as confirmed by the addressed past comments).
147-150: LGTM: Version parameter propagation correctly wired.The
env.KTOR_VERSIONparameter is now properly propagated from the version resolver via dependency parameter reference, and the snapshot dependency ensures the resolver runs first. This addresses the critical issue flagged in previous reviews.Also applies to: 152-158
187-190: LGTM: Regular samples also get proper version propagation.Like the plugin samples, regular samples now correctly receive
env.KTOR_VERSIONfrom the resolver via dependency parameter reference. This addresses the critical issue raised in earlier reviews.Also applies to: 198-204
312-328: LGTM: Efficient filtering eliminates O(n²) waste.The use of
zipto create pairs once, followed by simple filtering, is a significant improvement over the previous approach that repeatedly calledcreateEAPBuildType(). This addresses the performance concern raised in earlier reviews.
386-425: LGTM: Final composite cleanly aggregates all EAP validation.The top-level composite correctly depends on both the plugin and regular sample composites, providing a single orchestration point for the entire EAP validation workflow with appropriate notifications.
No description provided.