Conversation
Signed-off-by: Olivier Lamy <olamy@apache.org>
24dbf28 to
4de1dd1
Compare
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
There was a problem hiding this comment.
Pull request overview
Updates Maven’s logging stack to SLF4J 2.0.17 and migrates the Maven-provided “simple” binding from the SLF4J 1.7 StaticLoggerBinder mechanism to the SLF4J 2.x SLF4JServiceProvider mechanism.
Changes:
- Upgrade SLF4J from 1.7.36 to 2.0.17 and adjust related build/documentation references.
- Replace
StaticLoggerBinderwith a newMavenSimpleServiceProviderand service descriptor underMETA-INF/services/. - Move Maven’s customized simple logger classes from
org.slf4j.impltoorg.slf4j.simpleand update embedder wiring accordingly.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates bootstrap command to use a 3.10.x snapshot target directory. |
| pom.xml | Bumps slf4jVersion to 2.0.17 and adjusts SLF4J-related dependency metadata. |
| maven-slf4j-provider/src/test/java/org/slf4j/simple/MavenSimpleLoggerTest.java | Aligns test package with SLF4J 2.x simple package. |
| maven-slf4j-provider/src/main/resources/META-INF/services/org.slf4j.spi.SLF4JServiceProvider | Adds SLF4J 2 provider registration. |
| maven-slf4j-provider/src/main/java/org/slf4j/simple/MavenSimpleServiceProvider.java | Implements SLF4J 2 SLF4JServiceProvider for Maven’s simple logger. |
| maven-slf4j-provider/src/main/java/org/slf4j/simple/MavenSimpleLoggerFactory.java | Moves factory into SLF4J 2 simple package. |
| maven-slf4j-provider/src/main/java/org/slf4j/simple/MavenSimpleLogger.java | Moves logger into SLF4J 2 simple package. |
| maven-slf4j-provider/src/main/java/org/slf4j/impl/StaticLoggerBinder.java | Removes SLF4J 1.7 binder implementation. |
| maven-slf4j-provider/pom.xml | Updates unpacked source package paths and excludes SLF4J 2 SimpleServiceProvider. |
| maven-embedder/src/main/resources/META-INF/maven/slf4j-configuration.properties | Adds mapping for Maven’s new simple-factory package name. |
| maven-embedder/src/main/java/org/slf4j/simple/MavenSlf4jSimpleFriend.java | Moves helper into SLF4J 2 simple package. |
| maven-embedder/src/main/java/org/apache/maven/cli/logging/impl/Slf4jSimpleConfiguration.java | Updates import to new helper package. |
| apache-maven/src/main/appended-resources/licenses/unrecognized-slf4j-api-2.0.17.txt | Adds license text for the updated SLF4J artifact version. |
Comments suppressed due to low confidence (1)
pom.xml:372
- With
slf4jVersionupgraded to 2.0.17, the managedlogback-classicversion1.2.13is a SLF4J 1.7 binding and is not compatible with SLF4J 2.x (it will trigger binding/version mismatch warnings and won’t function as a provider). If Maven still intends to support the optional Logback binding path, updatelogback-classicto a SLF4J 2 compatible release (e.g., Logback 1.4+), or remove/replace this optional dependency to avoid advertising a broken combination.
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
<version>${slf4jVersion}</version>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-simple</artifactId>
<version>${slf4jVersion}</version>
</dependency>
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
<version>1.2.13</version>
<optional>true</optional>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
maven-embedder/src/main/resources/META-INF/maven/slf4j-configuration.properties
Show resolved
Hide resolved
Signed-off-by: Olivier Lamy <olamy@apache.org>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
pom.xml:371
- After upgrading to SLF4J 2.0.x,
logback-classic1.2.13 is an SLF4J 1.7-era binding and is not compatible with the SLF4J 2.x provider mechanism (and can also pull in an olderslf4j-apitransitively). Please bumplogback-classicto an SLF4J 2-compatible line (e.g., 1.3.x for Java 8) or otherwise adjust/remove this optional dependency to avoid runtime binding conflicts.
</dependency>
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
<version>1.2.13</version>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| @Override | ||
| public void initialize() { |
There was a problem hiding this comment.
MavenSimpleServiceProvider.initialize() only instantiates MavenSimpleLoggerFactory, but does not perform the SimpleLogger initialization that slf4j-simple typically requires. With SLF4J 2.x providers, the provider’s initialize() is the right place to call SimpleLogger.init() (and any required factory reset) so configuration/system properties are applied before loggers are used.
| public void initialize() { | |
| public void initialize() { | |
| SimpleLogger.init(); |
Following this checklist to help us incorporate your
contribution quickly and easily:
Note that commits might be squashed by a maintainer on merge.
This may not always be possible but is a best-practice.
mvn verifyto make sure basic checks pass.A more thorough check will be performed on your pull request automatically.
If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.