Repository navigation
Drop the dead unused suppressions from Spine and DokkaTools - #806
Merged
Merged
Conversation
These two are the only catalogues in `io.spine.dependency.local` whose every declared member is referenced, so their object-level `unused` entry has nothing left to suppress. `Spine` declares `group` and `toolsGroup` alone, with 11 and 17 references from outside the file. All of them live in `buildSrc`, which `migrate` copies wholesale, so the referencing files always travel with `Spine.kt`. `DokkaTools` declares `group`, `version` and `extensions`. `extensions` is used by `useDokkaWithSpineExtensions()` in `DokkaExts.kt`, and `group` and `version` are used by the `extensions` template. That chain ends at `dokka-setup.gradle.kts`, applied by id from `jvm-module.gradle.kts` and `kmp-publish.gradle.kts`. `Reflect` already has this exact shape without an `unused` entry. The other nine catalogues in the package keep theirs: their constants are genuinely unreferenced here, because downstream repos consume them. The `ConstPropertyName` entry stays in both files: the constant names are lowerCamelCase by the catalogue's own convention. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
There was a problem hiding this comment.
🟢 Approved
Both removals are supported by existing member references and leave executable code and dependency coordinates unchanged.
No open issues 🎉
What changed in this PR
Removes two obsolete unused suppressions from dependency catalogues distributed to downstream repositories, following #803.
Changes:
- Removes
unusedfromSpineandDokkaTools. - Preserves
ConstPropertyName, annotation formatting, and all dependency coordinates.
| File | Description |
|---|---|
buildSrc/src/main/kotlin/io/spine/dependency/local/Spine.kt |
Removes the obsolete group-constant suppression. |
buildSrc/src/main/kotlin/io/spine/dependency/local/DokkaTools.kt |
Removes the obsolete Dokka catalogue suppression. |
Review effort: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #803. That PR documented every object-level
@Suppressentryunder
buildSrc/src/main/kotlin/io/spine/dependency/local/with a/* rationale */comment, without adding, removing or reordering a singlesuppression name. Writing those rationales is what exposed that two
unusedentries had nothing left to suppress. This PR removes exactly those two.
Why these two, and only these two
The catalogues in this package are copied into every downstream repo by
./config/pull, so a constant with no reference insideconfigis theexpected state, not evidence of staleness. A class-level
unusedsuppression is dead only when every declared member is referenced — which
is what makes these two objects special. Nine other catalogues in the package
keep their
unusedentry for exactly this reason, and this PR leaves themuntouched.
Spinedeclares two members, both referenced:Spine.ktgrouptoolsGroupAll 28 live inside
buildSrc, whichmigratecopies wholesale — so thereferencing files always travel with
Spine.kt. The old rationale, "Somesubprojects do not use all the declared groups", could never apply: no
consumer can receive
Spine.ktwithout also receiving its referencers.DokkaToolsdeclares three members, all referenced:extensionsDokkaExts.kt:48, insideuseDokkaWithSpineExtensions()groupextensionstemplate, same fileversionextensionstemplate, same fileThe chain terminates at an entry point rather than trailing off into dead
code:
dokka-setup.gradle.kts:23callsuseDokkaWithSpineExtensions(), andthat precompiled script plugin is applied by id from
jvm-module.gradle.kts:43and
kmp-publish.gradle.kts:50.Reflectalready has this exact shape — onlyits
libreferenced across files, the rest consumed bylib's own template —and carries no
unusedentry.What is deliberately unchanged
ConstPropertyNamestays in both files. The names are lowerCamelCaseconst vals by this catalogue's own convention, so that entry is live.order was touched. The diff is two deleted lines.
@Suppress(...)keeps its multi-line form andtrailing comma, matching
BaseTypes.kt,Change.kt,TestLib.ktandReflect.kt. Collapsing it to one line would exceed detekt'sMaxLineLength: 100once the inline rationale is included.Severity
unusedis an IDE inspection. Neither detekt nor the Kotlin compiler enforcesit, so a stale entry costs nothing at build time and this change has no effect
on compilation. It is IDE-noise cleanup.
Verification
./gradlew detektpasses (:buildSrc:compileKotlinexecuted, not cached).confighas no rootversion.gradle.kts, so the version gate does not apply;buildis ambiguous here anddokkaGenerateis not registered, sodetektisthe gate. Reviewed by
spine-code-review,kotlin-engineer,review-docsanddependency-audit.Still open, not addressed here
ConstPropertyName, 5 withunused, andCoreJvmCompilerwithMemberVisibilityCanBePrivate. Cosmetic, and mixing it into a PR thatchanges which inspections are suppressed is the kind of churn that got
this work split out of Bump CoreJvm ->
2.0.0-SNAPSHOT.571#801.BaseTypesandChangehave no reference anywhere inthis repo, yet carry no
unusedentry at all — the mirror image of whatthis PR fixes. Pre-existing, and presumably fine in practice because
consumers use the distributed
buildSrc.🤖 Generated with Claude Code