eclipse-emf / org.eclipse.emf

Eclipse Public License 2.0
10 stars 13 forks source link

Migrate from log4j-1 to slf4j and remove unnecessary logging imports #17

Closed HannesWell closed 10 months ago

HannesWell commented 10 months ago

Log4J-1 used to be EOL, now it has been revived with reload4j, but nevertheless I think it is better to use a more active logging facade like slf4j. If you prefer log4j-2 would also be possible but I haven't checked yet if it log4j-2 API is available in the TP.

Importing org.apache.commons.logging was not necessary. Probably it was generated by XText. In this sense another remark: AFAIK Xtext currently still generates log4j-1 dependencies even for new projects, so in case the project is regenerated this should probably be considered.

merks commented 10 months ago

I wonder how compatible this is with all the older target platforms:

image

https://ci.eclipse.org/emf/job/all-target-platforms/

I suppose I can disable the xcore parts of the build as necessary...

HannesWell commented 10 months ago

I wonder how compatible this is with all the older target platforms:

As long as those TPs contain the slf4j-api bundle it should work. The usage is so simple that it should be compatible with pretty old versions of slf4j. At the moment the package org.slf4j is imported with version-range [1.7.0,3.0.0), but I can check if those those TPs only contain slf4j<1.7.

merks commented 10 months ago

I can also added an Orbit repository location too. Let's just go for it and deal with the follow up issues after...

HannesWell commented 10 months ago

Kepler (which looks like the oldest TP to me) contains slf4j 1.7.2 and the p2 metadata indicate a properly exported package org.slf4j, so this change should be compatible even with that. :)

image

merks commented 10 months ago

It didn't quite "just work" but it was easy to fix....