gnodet-bot commented on code in PR #2258:
URL: https://github.com/apache/maven-resolver/pull/2258#discussion_r4459903635
##########
maven-resolver-tools/pom.xml:
##########
@@ -204,6 +204,15 @@
<artifactId>maven-settings</artifactId>
<version>${maven3Version}</version>
</dependency>
+ <!-- maven-plugin-tools-api's httpclient dependency needs this to fetch
external javadoc package lists
+ (JavadocSite.createHttpClient). maven-resolver-transport-apache above
also depends on httpclient but
+ excludes commons-logging, and since it is declared first, that
exclusion otherwise wins the conflict
+ for the whole reactor. Declare it directly so the doclet run does not
fail with NoClassDefFoundError. -->
+ <dependency>
+ <groupId>commons-logging</groupId>
+ <artifactId>commons-logging</artifactId>
+ <version>1.2</version>
+ </dependency>
Review Comment:
⚠️ **Missing `<scope>runtime</scope>`**: `commons-logging` is only needed at
runtime for the javadoc doclet run (not at compile time), consistent with the
other build-only deps in this tooling module (e.g. `commons-lang3` right below
uses `<scope>runtime</scope>`, and the exec-maven-plugin uses
`<classpathScope>test</classpathScope>`). Adding the scope here prevents
`commons-logging` from accidentally leaking into compile-time classpaths of
consumers.
```suggestion
<dependency>
<groupId>commons-logging</groupId>
<artifactId>commons-logging</artifactId>
<version>1.2</version>
<scope>runtime</scope>
</dependency>
```
--
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]
