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://github.com/apache/ctakes/blob/837666cc2f8d25520255886e9525feeeb9510fbd/ctakes-assertion/src/main/java/org/apache/ctakes/assertion/medfacts/AssertionAnalysisEngine.java#L287
> - 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
