Hi Richard,
Many, many thanks for doing this. I gave it a pass and don't see anything that
looks like it might cause a problem. I will run some tests. My comments on
your comments:
- The lifecycle plugin in the root POM uses "version" instead of "versionRange"
in the main branch - the PR fixes that
-- That sounds good to me.
- 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.
- 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.
- 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?
- 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.
- 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.
Cheers,
Sean
________________________________
From: Richard Eckart de Castilho <r...@apache.org>
Sent: Friday, July 26, 2024 8:36 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 Sean,
i have set up an initial PR here:
https://urldefense.com/v3/__https://github.com/apache/ctakes/pull/21__;!!NZvER7FxgEiBAiR_!rOgnJ7H15Yz2Yhom0R9PmwVVqB8yO-_DzBI5-d-yDg0laTqSxcT1vW91vTLO1RYQk42IoHdn3V3MbcEQHKLmJw$
However, there are a few issues:
- The lifecycle plugin in the root POM uses "version" instead of "versionRange"
in the main branch - the PR fixes that
- The main branch still uses ContextSingletonBeanFactoryLocator which is not
part of Spring anymore and thus the Eclipse has compiler errors
- 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.
- 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.
- Calls to SLF4J in UIMA components should be replaced by `getLogger().xxxx` -
I did this in some cases
- 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
Cheers,
-- Richard
