Repository navigation
Document object-level @Suppress entries in dependency/local - #803
Merged
Merged
Conversation
All 15 artifact catalogues under `io.spine.dependency.local` now use the same documented multi-line `@Suppress` form, with a block comment stating why each inspection is suppressed. Previously ten of them carried the bare one-line form. The set of suppressed inspection names is unchanged in every file; this is a documentation-of-intent change only. 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
The changes are limited to documentation and formatting, preserve suppression behavior, and have no identified blocking issues.
No open issues 🎉
What changed in this PR
Standardizes object-level suppression documentation in the local dependency catalogues, following the cleanup deferred from #801.
Changes:
- Expands ten annotations into documented, multi-line form.
- Adds the missing
CoreJvmCompilerrationale and corrects wording inTime. - Preserves suppression names, ordering, dependency versions, and behavior.
| File | Description |
|---|---|
buildSrc/src/main/kotlin/io/spine/dependency/local/Validation.kt |
Documents suppression rationales. |
buildSrc/src/main/kotlin/io/spine/dependency/local/ToolBase.kt |
Documents suppression rationales. |
buildSrc/src/main/kotlin/io/spine/dependency/local/Time.kt |
Corrects rationale wording. |
buildSrc/src/main/kotlin/io/spine/dependency/local/TestLib.kt |
Documents naming suppression. |
buildSrc/src/main/kotlin/io/spine/dependency/local/Spine.kt |
Documents suppression rationales. |
buildSrc/src/main/kotlin/io/spine/dependency/local/Reflect.kt |
Documents naming suppression. |
buildSrc/src/main/kotlin/io/spine/dependency/local/Logging.kt |
Documents suppression rationales. |
buildSrc/src/main/kotlin/io/spine/dependency/local/CoreJvmCompiler.kt |
Documents unused suppression. |
buildSrc/src/main/kotlin/io/spine/dependency/local/CoreJvm.kt |
Documents object-level suppressions. |
buildSrc/src/main/kotlin/io/spine/dependency/local/Change.kt |
Documents naming suppression. |
buildSrc/src/main/kotlin/io/spine/dependency/local/BaseTypes.kt |
Documents naming suppression. |
buildSrc/src/main/kotlin/io/spine/dependency/local/Base.kt |
Documents suppression rationales. |
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.
What
Makes the class-level
@Suppressannotations acrossbuildSrc/src/main/kotlin/io/spine/dependency/local/consistent.Two styles had accumulated in that directory. Five files documented each
suppression inline with a block comment; ten still used the bare one-line form.
This PR converts the bare ones, so all 15 artifact catalogues now read the same
way:
@Suppress( "ConstPropertyName" /* We use a custom convention for artifact properties. */, "unused" /* Some subprojects do not use all Base artifacts. */, ) object Base {Follow-up to the review of #801, where this was raised as an optional nit and
explicitly kept out of that PR — expanding a
@Suppressis unrelated churn on aversion-pin diff.
Survey
masterCompiler,CoreJvmCompiler,DokkaTools,ProtoTap,TimeBase,BaseTypes,Change,CoreJvm,Logging,Reflect,Spine,TestLib,ToolBase,ValidationRationale wording
Reused verbatim from the files that already documented the same suppression
name, rather than invented per file:
ConstPropertyName→ We use a custom convention for artifact properties.MemberVisibilityCanBePrivate→ The properties are used directly by other subprojects.unused→ per-file, following the existing pattern: Some subprojects do notuse the Compiler directly. for the tools, Some subprojects do not use all
Time artifacts. for the version catalogues.
Two files that already used the multi-line form were brought fully into line:
Time.kt— a pure wording fix: it said We use custom convention…, so themissing article was added. All 15
ConstPropertyNamerationales in thepackage are now character-identical.
CoreJvmCompiler.kt— not a wording fix but a content addition: itsunusedentry was bare
"unused"while its other two entries were documented, so itgained a rationale and the trailing comma the other four files use.
Not changed
No suppression name was added, removed, or reordered. The set of suppressed
inspections is unchanged in effect in every file, including the entry orders that
differ between files (
Spine.ktlists"unused"first; most list"ConstPropertyName"first). This is a documentation-of-intent change only, sothe diff contains no version literal and no Maven coordinate.
Member-level suppressions are out of scope. The bare
@Suppress("unused")on the deprecated
typealias CoreJavainCoreJvm.ktis left as-is, eventhough this PR reformats that file's object-level annotation — it is the only
bare suppression left in the package. It is not stale (
CoreJavais retainedfor consumer build scripts), so documenting it is a reasonable separate change.
Sibling scopes are out of scope.
lib/,build/,test/,kotlinx/, andboms/hold 67 bare-form files against a single documented one(
lib/Protobuf.kt). The majority style there is the bare form, so convertingthem is a separate decision rather than a consistency fix, and far too large to
fold in here.
Follow-up observations
None of these are acted on here — dropping a suppression is a behaviour change
and does not belong in a documentation PR. Note that
unusedis an IDEinspection: neither
detektnor the compiler enforces it, so a stale entrycosts nothing at build time. These are IDE-noise cleanups.
Spine.kt'sunusedis dead. Its only members,groupandtoolsGroup, have 11 and 17 out-of-file references respectively — it is theone object in the package where every declared member is referenced
elsewhere, so nothing in the file can trigger
unused. It is dead inconsumer repos too, since the file travels there via
./config/pull. ItsConstPropertyNameentry is live (const val groupis lowerCamelCase).DokkaTools.kt'sunusedalso looks dead —extensionsis used atbuildSrc/src/main/kotlin/DokkaExts.kt:48, andgroup/versionarereferenced in the object's own string templates. (Not a file this PR
touches.)
ConstPropertyName, 5 withunused, andCoreJvmCompilerwithMemberVisibilityCanBePrivate. Deliberately untouched under the no-reorderconstraint, but now that the wording is uniform it is a cheap cosmetic
follow-up.
Every other suppression was checked and is live: the remaining nine
unusedentries each have at least one member with no references anywhere, all four
MemberVisibilityCanBePrivateentries guard members used only inside their ownobject, and all 15
ConstPropertyNameentries are needed.Verification
./gradlew detekt—BUILD SUCCESSFUL(the CI gate;confighas nobuildtask).buildSrc:compileKotlinsucceeds, so the annotations parse.git diff --checkclean; longest added line is 82 characters against a100-character limit.
@Suppressof everytouched file is identical at
origin/masterand atHEAD.catalogues.
🤖 Generated with Claude Code