Improve ComparatorRegistry API (reified helpers as members)#952
Conversation
Also: * Fix the argument check in `repeat`.
There was a problem hiding this comment.
Pull request overview
This PR refines the public API of ComparatorRegistry (and Indent) by relocating reified helper functions into their owning types, while also rolling in routine version/dependency/report updates and some build/coverage tooling maintenance.
Changes:
- Move reified
ComparatorRegistryhelpers (register/get/find/contains) from top-level extensions intoComparatorRegistrymembers. - Move
Indent.repeat/Indent.atLevelfrom extensions intoIndentmembers (and correct the argument validation/message). - Bump published snapshot version and refresh dependency versions/reports (including Jackson
2.22.1).
Reviewed changes
Copilot reviewed 18 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| version.gradle.kts | Bumps versionToPublish snapshot. |
| docs/dependencies/pom.xml | Updates generated dependency POM versions (incl. Jackson, Spine artifacts). |
| docs/dependencies/dependencies.md | Regenerates dependency/license report with updated versions/timestamp. |
| buildSrc/src/main/kotlin/io/spine/gradle/testing/TestKitCoverage.kt | Refactors agent coordinate usage for TestKit coverage wiring. |
| buildSrc/src/main/kotlin/io/spine/gradle/testing/SpineCompilerCoverage.kt | Adds new helper to capture JaCoCo exec data from forked Spine Compiler JVMs. |
| buildSrc/src/main/kotlin/io/spine/gradle/report/coverage/SiblingCoverage.kt | Adjusts visibility/API to share the “consumes binary reports” predicate. |
| buildSrc/src/main/kotlin/io/spine/gradle/report/coverage/KoverConfig.kt | Extends root Kover aggregation to include additional exec data sources and updates Gradle dependency notation. |
| buildSrc/src/main/kotlin/io/spine/dependency/test/Jacoco.kt | Centralizes JaCoCo agent coordinates as Jacoco.agent. |
| buildSrc/src/main/kotlin/io/spine/dependency/local/Validation.kt | Updates local dependency version constant. |
| buildSrc/src/main/kotlin/io/spine/dependency/local/ToolBase.kt | Updates local dependency version constants. |
| buildSrc/src/main/kotlin/io/spine/dependency/local/Time.kt | Updates local dependency version constant. |
| buildSrc/src/main/kotlin/io/spine/dependency/local/Logging.kt | Updates local dependency version constant. |
| buildSrc/src/main/kotlin/io/spine/dependency/local/CoreJvm.kt | Updates local dependency version constant. |
| buildSrc/src/main/kotlin/io/spine/dependency/local/Compiler.kt | Updates fallback compiler versions. |
| buildSrc/src/main/kotlin/io/spine/dependency/local/Base.kt | Updates local dependency versions used for build/script alignment. |
| buildSrc/src/main/kotlin/io/spine/dependency/lib/Jackson.kt | Bumps Jackson version to 2.22.1. |
| base/src/main/kotlin/io/spine/string/Indent.kt | Moves repeat/atLevel into Indent and fixes validation bug. |
| base/src/main/kotlin/io/spine/compare/ComparatorRegistry.kt | Moves reified helper functions into ComparatorRegistry as members. |
| .idea/misc.xml | Removes committed IDEA project-local settings file. |
| .gitignore | Stops re-including .idea/misc.xml and adds ignores for Claude Code local files/dirs. |
| .claude/settings.json | Updates Claude Code settings (plans directory and allowed actions). |
Files not reviewed (1)
- .idea/misc.xml: Generated file
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19f296753b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Address Copilot review on PR #952: constrain `register<reified T>` to `<reified T : Any>` to match `get`/`find`/`contains` (a class literal requires a non-null type), and fix "no a comparator" -> "no comparator" in both `get` overloads. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #952 +/- ##
=======================================
Coverage 94.02% 94.02%
=======================================
Files 192 192
Lines 4187 4187
Branches 346 345 -1
=======================================
Hits 3937 3937
Misses 149 149
Partials 101 101 🚀 New features to boost your workflow:
|
Summary
Improves the
ComparatorRegistryAPI and folds in the routine build/dependencymaintenance that accumulated on the branch. As requested, all commits on the
branch are included in this PR.
What changed
API / code
ComparatorRegistry— the fourreifiedinline helpers (register,get,find,contains) move from top-level extension functions intomembers of the
ComparatorRegistryobject, alongside theirClass-basedcounterparts. Call syntax is unchanged (e.g.
ComparatorRegistry.get<Foo>()).Because these helpers are
inline, removing the old top-level forms iseffectively binary-safe for already-compiled callers.
Indent—repeatandatLevellikewise move from extension functionsto members. This also fixes a latent bug in the old
repeat: it guardedrequire(size >= 0)(always true, sincesize > 0is enforced atconstruction) and interpolated
$sizeinstead of the argument; the membernow validates and reports the argument
n.Dependencies / build
2.22.1.2.42.0— pinned as the last release compatible withJava 17 (
2.43.0requires JDK 21).configsubmodule and sync the distributedbuildSrc(Kover/coverage configuration, dependency
localobjects).docs/dependencies/.Versioning
2.0.0-SNAPSHOT.423→2.0.0-SNAPSHOT.424.Commits
Meaningful:
Bump version -> 2.0.0-SNAPSHOT.424Transform extension functions in to members(Indent)Bump Jackson -> 2.22.1Constrain reified register to Any; fix get KDoc grammar(review follow-up)Bump ErrorProne -> 2.42.0Routine sync:
Update config×2,Update dependency report(s)×4.Validation
The local pre-PR checklist ran green on the
ComparatorRegistry/Indentchanges:
./gradlew build dokkaGenerateon JDK 17 — PASS (incl. the Kovercoverage gate and Dokka link resolution); reviewers
spine-code-reviewAPPROVE,
kotlin-engineer/review-docsAPPROVE WITH CHANGES, noMust-fix. The later dependency bumps (Jackson, ErrorProne) and review
follow-ups are covered by CI on the branch head.
Notes for reviewers
ae91fe61a7):ComparatorRegistry.registeris now
<reified T : Any>(matching its siblings), and the relocatedget()KDoc grammar ("no a comparator" → "no comparator") is corrected.
Indentbinary compatibility (decided): dropping the non-inlineIndentKt.repeat/atLevelfacade is a binary break for pre-compiled callers.Accepted as intentional for the
2.0.0major line — SNAPSHOTs don't promisecross-version binary compatibility.
Indent.atLevel(l: Int)uses a single-letter parameter;levelwould read better.