lets clean up logging

Tags: #<Tag:0x00007fe2da623058> #<Tag:0x00007fe2da622c20>

Hi there!

I think there is a logging confusion mixup going on in openmrs-core. Please help me understand if we should clean this up or if I am wrong here :slight_smile:

In the openmrs-core root pom we exclude the apache commons-logging dependency coming from springframework. I assume this is because we want to use the logging facade https://www.slf4j.org/ that we import which in turn delegates to the log4j as the logging implementation. This all seems like we try to use springs approach documented here http://docs.spring.io/spring/docs/4.1.4.RELEASE/spring-framework-reference/htmlsingle/#overview-not-using-commons-logging

But if you search in the code for apache.commons.logging you find quite a bit of classes that depend on it. For example ConceptReferenceTermEditor; there are many more.

So

  1. Should I create an issue with fixed in release 2.2.0 that will replace all occurrences of the apache.commons.logging with slf4j AND eliminate the apache.commons dependency that seems to slip in somewhere by another dependency
  2. Add an entry to the https://wiki.openmrs.org/display/docs/Java+Conventions that tells devs to use the slf4j
  3. Add a rule in Checkstyle/PMD that will flag the use of apache.commons.logging

I support all the 3 steps.

Thanks for cleaning this up @teleivo.

One concern–thoughts on how this will affect modules that might be making the same mistake, and may be relying on the apache.commons dependency that manages to slip in somewhere? If it may break them, we’ll need to document the fix for module owners.

Take care, Mark

Ah, it looks like you are already thinking about module issues in the other thread that I’m just reading now…

Nice proposal! :smile:

@teleivo after merging your pull request, i just noticed that our commons.logging comes from jcl-over-slf4j-1.6.0.jar Is it the one mentioned in the above spring link?