vlsi commented on code in PR #129:
URL: https://github.com/apache/xalan-java/pull/129#discussion_r1407441482
##########
serializer/src/main/java/org/apache/xml/serializer/Version.java:
##########
@@ -55,7 +55,16 @@ public final class Version
private static void readProperties() {
Properties pomProperties = new Properties();
- try (InputStream fromResource =
Version.class.getClassLoader().getResourceAsStream(POM_PROPERTIES_PATH)) {
+ ClassLoader classLoader = Version.class.getClassLoader();
+ if (classLoader == null) {
+ // Oops! Someone put Xalan is on the bootstrap class loader (BCL) -> fall
+ // back to the system class loader, because there is no Classloader
+ // instance for the BCL (native code). Due to class loader hierarchy,
+ // however, the resource will also be found when asking for it from a
+ // level below the BCL.
+ classLoader = ClassLoader.getSystemClassLoader();
+ }
+ try (InputStream fromResource =
classLoader.getResourceAsStream(POM_PROPERTIES_PATH)) {
Review Comment:
> You are not even working actively on the Maven branch
It is not important if I dislike Maven or not.
Please check https://www.apache.org/foundation/how-it-works/#meritocracy. I
have suggested PRs for the other topics, I have answered users' questions on
the mailing list.
> Earlier we talked about test automation several times, and nowhere did
anyone tell me to add test plugins or dependencies before the Maven cutover
May I suggest that you should pioneer by adding the first JUnit5 test right
within the PRs you suggest?
Sure the others will see how to add them and it is likely they add more.
> Are you in any position to speak authoritatively and command me to do such
things?
Wow. You suggest code changes, and I suggest that you should cover your
changes with tests.
Sure you can ignore my suggestions based on whatever reasons. Just be fair
and don't say I am asking too much.
Previously you just said ["that is a separate (huge)
PR"](https://github.com/apache/xalan-java/pull/129#discussion_r1407046638)
which is either a misunderstanding (so I listed a step-by-step for adding unit
test) or a deliberate lie.
Sure I never asked you to add integration tests to xalan-test-whatever, so
do not put it as if "you already covered everything while I am requiring huge
additional efforts".
> My suggestion is Spock
Spock is tied to Groovy, and Groovy has issues running with modern Java
versions. For instance, testing with Java 22/23/.. would be problematic as in
https://github.com/apache/jmeter/issues/6114#issuecomment-1825534617
I suggest avoiding Spock and Groovy unless they provide significant
advantages over the regular Java/Kotlin code.
> Theoretically assuming for a minute, I would add tests separately from
xalan-test, what would be the consequence?
I will send you an email in 3 years. Hopefully "maven cutover" will be
merged by then, and I bet the tests will use JUnit Platform + JUnit Jupiter.
> They already existed without any test coverage. I merely ported them to
the Maven build
The absence of tests should not be an excuse for not adding tests for the
code you change.
It might be that innocently-looking changes introduce subtle bugs.
>I wanted to avoid code generation magic in favour of simple resource
filtering
"code generation" is the same as resource filtering. I do not see why you
call "replacing a variable in a file" with "magic". Sure replacing a variable
is not magic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@xalan.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@xalan.apache.org
For additional commands, e-mail: dev-h...@xalan.apache.org
