kriegaex commented on PR #133: URL: https://github.com/apache/xalan-java/pull/133#issuecomment-1836446212
> I say: "This looks great to me". I was OK merging this from the beginning. I read that. But you did not say it was OK to merge. And you wrote that sentence **before** you started flooding me with review comments indicating otherwise. > here's a diff that replaces Mockito with a line of Java code keeping the same test coverage. Am I missing something? Yes, you are missing that test coverage is not the whole story here. You pulled the `getPropertiesStream()` call out of the method handling possible I/O (or other runtime) exceptions. I.e., if anything goew wrong during that I/O operatione, e.g. because our build is broken and the resource file is missing in the JAR, then we have an unhandled exception directly when loading the class. My solution keeps Xalan running and returning version 0.0.0, even if something goes wrong there. It is not worth killing a whole application with an uncaught error for a minor tool class. -- 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
