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: 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