slachiewicz opened a new pull request, #1356:
URL: https://github.com/apache/maven-javadoc-plugin/pull/1356

   The `maven-compat` dependency here carries a `@TODO remove!!` and nothing in 
the plugin imports anything from it. Removing it alone does not work, and the 
reason is not in this plugin.
   
   ### Why it could not simply be deleted
   
   `maven-core` **3.6.3** ships `DefaultProjectBuildingHelper` with a field of 
the legacy `org.apache.maven.repository.RepositorySystem`, whose only 
implementation — `LegacyRepositorySystem` — lives in maven-compat. So any test 
that looks up a Mojo needing `ProjectBuilder` fails without it, with zero 
compat code in the plugin:
   
   ```
   No implementation for org.apache.maven.repository.RepositorySystem was bound.
     while locating org.apache.maven.project.DefaultProjectBuildingHelper
   ```
   
   `maven-core` changed that field to 
`org.apache.maven.bridge.MavenRepositorySystem`, which lives in maven-core 
itself.
   
   ### The version is 3.9.12 exactly, not "3.9.x"
   
   Worth being precise, since this sets a floor. Commit `3afbdb8f76` ("Use 
MavenRepositorySystem in ProjectBuildingHelper instead of deprecated 
RepositorySystem", #11358); `git tag --contains` gives `maven-3.9.12` as the 
first release carrying it. Confirmed against the bytecode:
   
   ```
   maven-core 3.9.11   org.apache.maven.repository.RepositorySystem 
repositorySystem
   maven-core 3.9.12   org.apache.maven.bridge.MavenRepositorySystem 
repositorySystem
   ```
   
   So `mavenVersion` moves to **3.9.12** — the earliest release where this 
holds — rather than a rounder number that would raise the floor further than 
necessary. `resolverVersion` moves to **1.9.25**, which is what Maven 3.9.12 
itself ships, rather than a version chosen independently.
   
   ### This is a project decision, not just a refactor
   
   `<prerequisites><maven>${mavenVersion}</maven></prerequisites>` reads that 
property, so this **raises the minimum Maven for consumers from 3.6.3 to 
3.9.12**. It wants a release-note entry and your agreement, not just a review 
of the diff.
   
   There is a legitimate alternative: **do nothing.** The dependency is 
`test`-scoped, never ships, and costs users nothing. If the project is not 
ready to move the floor, leaving it is defensible — but then the `@TODO 
remove!!` should say so, rather than implying someone forgot. The precedent for 
moving is `maven-plugin-plugin`, which removed its compat dependency in 
MPLUGIN-519/520 by raising its prerequisite.
   
   ### Verification
   
   `mvn verify`: **69 tests, 1 failure — identical to `master` under the same 
command.** `JavadocReportTest.testJavadocResourcesWithExcludes` already fails 
on an untouched checkout in my environment, and the failure set is the same 
before and after, so this change is neutral to it.
   
   Ran `verify` rather than `test` deliberately: on a sibling plugin, removing 
maven-compat passed all tests while breaking the build, because a dependency 
was reaching the compile classpath through it and only the dependency analysis 
in `verify` catches that.
   
   Part of a survey of which projects still declare `maven-compat` versus which 
genuinely need it. This one needed only the baseline.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to