kriegaex commented on code in PR #125:
URL: https://github.com/apache/xalan-java/pull/125#discussion_r1401430440
##########
serializer/src/main/java/org/apache/xml/serializer/Version.java:
##########
@@ -20,33 +20,97 @@
*/
package org.apache.xml.serializer;
+
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.Properties;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
/**
* Administrative class to keep track of the version number of
* the Serializer release.
* <P>This class implements the upcoming standard of having
- * org.apache.project-name.Version.getVersion() be a standard way
- * to get version information.</P>
+ * org.apache.project-name.Version.getVersion() be a standard way
+ * to get version information.</P>
* @xsl.usage general
*/
public final class Version
{
+ private static final String POM_PROPERTIES_JAR =
"org/apache/xml/serializer/version.properties";
+ private static final String POM_PROPERTIES_FILE_SYSTEM =
"serializer/target/classes" + POM_PROPERTIES_JAR;
+ private static final String VERSION_NUMBER_PATTERN =
"^(\\d+)[.](\\d+)[.](D)?(\\d+)(-SNAPSHOT)?$";
+ private static final String NO_VERSION = "0.0.0";
+
+ private static String version = NO_VERSION;
+ private static int majorVersionNum;
+ private static int releaseVersionNum;
+ private static int maintenanceVersionNum;
+ private static int developmentVersionNum;
+
+ private static boolean snapshot;
Review Comment:
The class in `serializer` is final, because I inherited it that way. But
that can be changed. As it is impossible to override static methods and use
polymorphism like with instance methods, it is not so super easy to avoid
duplication 100%. That is why e.g. in class `XSLProcessorVersion extends
Version` I had to redefine `getVersion()` to make sure that it calls its own
`getProduct()` method and not the one from its superclass. Again, a singleton
pattern would be nicer here, but I am not going to do that in this PR. I want
to port something, not completely refactor it. Plus, the static methods are
public API, we would have to keep them and make them call something like
`getInstance().myMethod()`. Then again, we would have the problem that in a
subclass for the static methods there would not be any polymorphism, and the
static method `getInstance()` or static field `INSTANCE` would be pulled from
the super class. We are not on a green field here, we are dealing with public
classes
and public static methods.
As for the dependency of `xalan` on `serializer`, it is indeed defined in
the 2.7.1 POM. But I had checked before on Maven Central and of course used the
latest version 2.7.3, which has no such POM and therefore no such dependency
defined. As POMs for Ant projects are always created somewhat synthetically, as
a total Xalan newbie I had no way of knowing if the currently defined
dependency in the Maven branch ought to be an optional or fixed dependency. I
did not bother to analyse the code to find out, because I was focusing on
porting the version classes only.
So can we not over-engineer this thing here? I already spent an order of
magnitude more time discussing the PR than to implement it. These cycles are
lost to more valuable support work I could have done for Joe with his open
Maven questions.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
