kriegaex commented on code in PR #129:
URL: https://github.com/apache/xalan-java/pull/129#discussion_r1409528027
##########
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.
Read again what you just quoted. I said you are _not actively working on
it_, but commanding around others who do. The main point was not about whether
you like it or not, but if you are helping or not.
> Please check https://www.apache.org/foundation/how-it-works/#meritocracy.
Stop lecturing me.
> > test automation (...) 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.
Fine, done: #133. Not just unit tests but also integration tests, because if
I start doing that, I want to do it right.
> I listed a step-by-step for adding unit test
You do not need to teach me how to add or write either unit or integration
tests. Like I said, I know how to do it. The debate was about whether it makes
sense to add them before or after the Maven cutover. Why do I have to repeat
myself so many times? Sometimes I feel like I am talking to someone suffering
from amnesia.
> > My suggestion is Spock
>
> Spock is tied to Groovy, and Groovy has issues running with modern Java
versions.
That is ridiculous! I have heavily used Spock in many large-scale commercial
projects (e.g. bank and telco IT), and that was never an issue.
> For instance, testing with Java 22/23/..
Hilarious! The current Java release is 21, and both Groovy and Spock are
under active development and regularly updated to work with current JDK
versions. It will be the same here. Save your pseudo arguments for someone
else, please. I want to remind you that until I personally recently added the
capability to compile on JDK 9+ (including 21), this project only compiled on
JDK 8. And now you start complaining about 22 or 23. If this is your biggest
problem, I am happy for you, because then you do not have any real ones.
> I suggest avoiding Spock and Groovy unless they provide significant
advantages over the regular Java/Kotlin code.
There are significant advantages. The BDD test DSL makes tests more
readable, so they may serve as a living specification. I often discussed and
designed Spock tests in the past with non-technical stakeholders, because they
could understand the basic structure and I was able to code specifications (and
tests) directly from their requirements. BTW: Groovy, in contrast to Kotlin, is
a 99% super set of Java, i.e. every Java developer can simply start writing
"Java-ish" Spock tests and learn Groovy syntax step by step. In 90% of the
commercial projects I used Spock in, the code under test was written in Java,
not in Groovy - only the tests were. Spock comes with a build-in mock
framework, usually there is no need to use Mockito or anything else on top of
it. I can express the same things with way less code, which not only saves time
while writing tests but also is more readable and more easily maintainable. I
know both JUnit and Spock and can compare them to each other very well from
experience. Can you?
> I will send you an email in 3 years. Hopefully "maven cutover" will be
merged by then
Your sarcasm is unhelpful. Even if I would not have volunteered to
temporarily support Joe in his endeavour and he would be all on his own -
obviously you have no intention to support him, only to crisicise everyone
involved - he would be finished way sooner than that.
> I bet the tests will use JUnit Platform + JUnit Jupiter.
Probably you are right in this respect, because that is what I used in #133,
despite knowing that it is a way worse choice that Spock would have been. But I
do not feel so inclined to waste more of my time with fruitless debates with
you, @vlsi.
> > 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.
I was not _excusing_ anything, I was _explaining_ something. Like I said, I
would have preferred not to open another front in this war (Maven cutover PR)
and rather win it battle by battle. Chances are that reviewers will complain
that too many new topics are contained in one big PR in the end. But alas, now
you got what you want. The whole Xalan team will have to deal with it.
> It might be that innocently-looking changes introduce subtle bugs.
I know that and agree. Again, it was not about _if_ but about _when_ to add
tests.
--
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
