SOLR-18296: DAGP external-lib api promotions — ILLUSTRATIVE, not for merge (part 3/3)#4614
Draft
serhiy-bzhezytskyy wants to merge 4 commits into
Draft
SOLR-18296: DAGP external-lib api promotions — ILLUSTRATIVE, not for merge (part 3/3)#4614serhiy-bzhezytskyy wants to merge 4 commits into
serhiy-bzhezytskyy wants to merge 4 commits into
Conversation
Completes the dependency-analysis follow-up deferred from the Gradle 9 upgrade (SOLR-18289), which dropped the Gradle-9-incompatible ca.cutterslade.analyze plugin. Adds the Dependency Analysis Gradle Plugin (com.autonomousapps build-health) configured to REPORT (warn), not fail — its advice is guidance for human review, not enforcement. Applies only the uncontroversial advice: - removal of genuinely-unused dependency declarations - a few test-scope corrections (implementation -> testImplementation) Deliberately NOT included here (left for separate case-by-case review): - implementation -> api promotions (DAGP is aggressive here; per dev@ these need human judgement, as liberal api is viral/transitive) Also forces kotlin-metadata-jvm to match Solr's Kotlin so DAGP can analyze the :solr:ui (Compose/KMP) module. Lockfiles regenerated.
Applies DAGP's api advice ONLY for inter-module project() dependencies that are genuinely part of the consuming module's public API (e.g. solr-core's public types expose solrj), where 'api' is correct so downstream modules compile. External-library api promotions are intentionally excluded here (see part 3/3); per dev@ feedback those need case-by-case judgement rather than blanket promotion. Builds on apache#4612. Lockfiles regenerated.
…ons (part 3/3) Shows the ~76 external-library implementation->api promotions DAGP advises, for discussion only. Per dev@ feedback these should NOT be applied wholesale: liberal 'api' is viral/transitive, leaking these deps onto every downstream consumer for often-trivial exposure. Concrete example of the harm: DAGP also advised promoting jersey-core-server to api, but that FAILS to resolve — its version comes from the jersey BOM (declared implementation), so as 'api' it has no version. Excluded here. This is exactly the kind of breakage liberal api promotion causes. Builds on apache#4613. Presented so reviewers can pick any genuinely-public ones case-by-case rather than accept/reject the whole set.
github-actions
Bot
added
documentation
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
1 participant
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.
Draft — NOT for merge. Part 3 of 3, for discussion only. Builds on #4613 → #4612.
This shows the ~76 external-library
implementation -> apipromotions DAGP suggests (slf4j, guava, junit, jakarta.*, jackson, log4j, opentelemetry, …), so the full picture is visible as code rather than a list.I do not recommend merging this as-is. Per @dsmiley's point on the dev@ thread: liberal
apiis viral/transitive — it leaks each of these onto every downstream consumer's compile classpath, often for trivial exposure. The right call is human judgement on which (if any) are genuinely part of Solr's public contract.Concrete evidence this advice is harmful, not just stylistically undesirable: DAGP also advised promoting
jersey-core-servertoapi, but doing so fails to resolve — its version comes from the jersey BOM (declaredimplementation), so asapiit has no version. I excluded it here to make the branch build. That's a real example of api-promotion breaking the build.Proposed use: rather than accept/reject the whole set, pick any deps that are genuinely public API and I'll fold just those into #4613. Everything else stays
implementation.Layering: #4612 (safe advice, ready) → #4613 (inter-module project() api, draft) → this (external api, illustrative).
AI-assisted, human-reviewed per AGENTS.md.