ctubbsii commented on a change in pull request #1314: ZOOKEEPER-3791: 
Miscellaneous Maven fixes
URL: https://github.com/apache/zookeeper/pull/1314#discussion_r406569648
 
 

 ##########
 File path: zookeeper-it/pom.xml
 ##########
 @@ -63,15 +63,17 @@
   </dependencies>
 
   <build>
-    <sourceDirectory>src</sourceDirectory>
-    <plugins>
-      <plugin>
-        <groupId>org.apache.maven.plugins</groupId>
-        <artifactId>maven-javadoc-plugin</artifactId>
 
 Review comment:
   I verified the contents of the compiled jar were identical to what was there 
before, and no Java classes or packages were changed.
   
   However, with my changes, the javadoc and source jar are now correctly 
containing what you would expect them to contain based on the contents of the 
main jar (and using correct path names to sources/javadoc files).
   
   Since the tests were moved to the `src/main/java` directory to achieve this, 
the previously built 'test-jar' artifact was skipped (because it would have 
resulted in an empty jar), and the surefire plugin needed to be reconfigured to 
execute the tests from the directory in which they are located. Having an 
integration-test module with tests in `src/main/java` is common as an 
alternative to creating a test-jar, especially if the contents of the jar are 
likely to be reused for downstream test code or benchmark code. I actually 
think that ZK's execution of `maven-jar-plugin:test-jar` in the parent POM for 
all modules is a bit excessive (I think only zookeeper-server module actually 
needs to create one), but that's something to be addressed separately.
   
   The only issue I can think of at all regarding any of this is that the 
test-jar is now removed for the `zookeeper-it` module... but I can't imagine 
anybody actually depended on that. And, if they did, it would only have been 
for test code of their own, and they can use the main jar artifact for 
`zookeeper-it` instead.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to