> The main branch still uses ContextSingletonBeanFactoryLocator
...
It looks like I had made a replacement a while ago for other instances of this
problem, but had somehow missed those two tests. Consider it done.
> For example in AssertionAnalysisEngine:
Oh, those. From the days of yor. Garbage imo. I get rid of them whenever I
find them.
>should be replaced by `getLogger().xxxx`
Right now I want to get vulnerabilities remediated and then get a release out
with that focus. After that, some "play nice, refactor" release involving
things like using uima api getLogger() as well as code consolidation, generics
(how archaic are we?), modern collections and streams ...
Cheers,
Sean
________________________________
From: Richard Eckart de Castilho <r...@apache.org>
Sent: Monday, July 29, 2024 4:09 PM
To: dev@ctakes.apache.org <dev@ctakes.apache.org>
Subject: Re: SLF4J instead of Log4J at the API level? [EXTERNAL]
* External Email - Caution *
Hi,
> On 29. Jul 2024, at 12:02, Finan, Sean
> <sean.fi...@childrens.harvard.edu.INVALID> wrote:
>
> - The main branch still uses ContextSingletonBeanFactoryLocator which is not
> part of Spring anymore and thus the Eclipse has compiler errors
> -- I can look for an appropriate replacement.
I briefly googled and I'm note sure if there is a proper direct replacement for
this. It looks like you're still using XML-based Spring configuration there.
Maybe using Spring Java-Config would be more sensible nowadays.
> - Many calls to the logging use String.format or similar means to construct
> the logged string. This can be done better using the SLF4J API,
> e.g. `debug("{} is less than {}", x, y)` or `error("This is an error, ex)`.
> I did not upgrade all these calls.
> -- Though I like to be a boy scout (and I noticed your if/then braces!), I
> don't expect anybody to make this code minty-fresh. It is nice that slf4j
> has this capability.
Maybe if I'm bored ;) Or maybe somebody else feels like it.
> - I commented out some calls to directly configure the logging system -
> instead there should be `log4j2-test.xml` files or similar configuration
> files for slf4j-simple used for testing.
> -- I couldn't find these in the modified files. Do you recall where they
> were - or for that matter, why they were?
For example in AssertionAnalysisEngine:
https://urldefense.com/v3/__https://github.com/apache/ctakes/blob/837666cc2f8d25520255886e9525feeeb9510fbd/ctakes-assertion/src/main/java/org/apache/ctakes/assertion/medfacts/AssertionAnalysisEngine.java*L287__;Iw!!NZvER7FxgEiBAiR_!qx0-fe8dC60hk-itqClF2maIsPjLQh0tOznHEcnBrbH3lwJf_fWe9Q14Y5vuUljMFP4eAk2YyBH6oTe3Cw2edQ$
> - Calls to SLF4J in UIMA components should be replaced by `getLogger().xxxx`
> - I did this in some cases
> -- Here I am ignorant. What is the advantage, and do you think that it is
> important enough to warrant a global find/replace? If so then I can give it
> a shot.
Since UIMA already supplies a logging mechanism for the components, I would
tend to using it. Internally, that is by default also routed through SLF4J.
Theoretically, a user of the components might re-configure UIMA to route the
logging differently for their particular environment. You could also argue
that you prefer using the same logging API throughout the entire code. So while
I say "should", it is arguable and you might come to a different conclusion
than I do.
> - It would IMHO be a good idea to introduce the maven-dependency-plugin to
> ensure that all packages used have direct non-transitive Maven dependencies
> and there are no unused dependencies
> -- Are you talking about the analyze goal? I suppose that we could add it
> to a phase for automatic execution.
Yep, that one exactly.
-- Richard
