Migrate to SLF4J for logging #1120
Conversation
lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java
Outdated
Show resolved
Hide resolved
lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java
Outdated
Show resolved
Hide resolved
| @@ -127,8 +127,7 @@ private static Provisioner forConfigurationContainer(Project project, Configurat | |||
| if (!projName.isEmpty()) { | |||
| projName = projName + "/"; | |||
| } | |||
| logger.log( | |||
| Level.SEVERE, | |||
| logger.error( | |||
There was a problem hiding this comment.
Perhaps use a {} placeholder here instead of mavenCoords
|
@jamietanna can you address the points @lutovich raised above? |
I'd kept this in line with the existing code to do this, as I'd assumed that there was a reason behind it. Looking at the code for I'm happy amending it, as it then looks more common for SLF4J, and that can then perform the |
As noted in diffplug#1116, it'd be ideal if we moved to SLF4J as our logging interface. We can do this pretty easily, making sure we remove our `package-list` reference, as well as making sure the SLF4J API is only used for compilation. We also need to map the error logging levels that most closely mirror what `java.util.logging` uses. Closes diffplug#1116.
As it removes unnecessary String concatenation at runtime, if the log levels aren't enabled.
As SLF4J allows passing in a `Class<?>`, instead of a `String` for the class name, we can simplify interactions with the logger creations.
|
If we're adopting log4j, let's do it canonically. Also take a look at "Perhaps use a {} placeholder here instead of mavenCoords". Thanks! |
jamietanna
force-pushed
the
feature/slf4j
branch
from
d387c37 to
85d15b5
Compare
|
Done, thanks for the speedy reply! |
|
Thanks! In the future, please add new commits instead of force-pushing, easier to review :) |
|
Sorry about that. I would've |
|
Looks good. Thanks for addressing my comments! |
|
Thanks a ton for taking on the SLF4J migration @jamietanna! |
|
Published in |
Closes #1116.