Use real logger slf4j #179
Conversation
** Because we only use this with maven and to be in line with maven, use 1.7.36 since that is what maven is still using.
| <dependency> | ||
| <groupId>org.slf4j</groupId> | ||
| <artifactId>slf4j-api</artifactId> | ||
| <version>1.7.36</version> |
There was a problem hiding this comment.
Its based on maven logging which is slf4j 1 still. Since this is used in the formatter maven plugin it needs to align there. However, it could move to 2 here as long as we use caution on the formatter plugin to stick to 1. Not sure if this is used outside that plugin thus the reason to just use version 1.
There was a problem hiding this comment.
So, this is just dependency convergence with one of the maven provided plugins, then? Does this cause an actual breakage in logging, or is this just being done "just in case"?
My concern here is that the logging framework provided by Maven is pretty opaque to plugins. There's no maven BOM containing provided libraries for us to import, which would make this a lot more convenient. And there's no guarantee that any version of slf4j will be provided by Maven in each release. So, we really have to make assumptions, and that makes me uncomfortable.
One option is to keep using slf4j2 here, and leave the responsibility for dependency convergence between Maven and this library to the formatter-maven-plugin. The formatter-maven-plugin can add an exclusion for slf4j coming in transitively from this plugin, and it can declare slf4j with the correct version as provided from Maven.
There was a problem hiding this comment.
I'm ok with making this module go to slf4j2 and just protecting on the plugin. That at least clears up concern here. On the plugin we are not providing any logging provider but we are using the apis. So it does matter there as slf4j completely changed their detection logic but agree that can be addressed there so this repo is cleaner since it doesn't matter here.
fixes #176
Also has renovate file, ignore it. That is for me, due to not agreeing, I'm using renovate and this is a small trade off. Renovate is not on upstream but config file needed for me so I'm not fighting moving commit back and forth.