[GitHub] [zookeeper] ctubbsii commented on a change in pull request #1508: ZOOKEEPER-3973: Add GitHub Actions for CI

[GitBox <gi...@apache.org>]

ctubbsii commented on a change in pull request #1508:
URL: https://github.com/apache/zookeeper/pull/1508#discussion_r517201704



##########
File path: pom.xml
##########
@@ -780,15 +780,6 @@
             <includeTestResources>false</includeTestResources>
             <includeTestSourceDirectory>true</includeTestSourceDirectory>
           </configuration>
-          <executions>
-            <execution>
-              <id>checkstyle</id>
-              <phase>validate</phase>
-              <goals>
-                <goal>check</goal>
-              </goals>
-            </execution>
-          </executions>

Review comment:
       This was removed because it literally did nothing. It defined an execution inside the `build/pluginManagement/plugins` section, and was never activated in a `build/plugins` section. The `<pluginManagement />` element is a way of early binding configuration to a plugin that is specified later. But, since the only execution of this plugin is on the command-line and not in a `build/plugins` section elsewhere in the `pom.xml` file, this literally did nothing.
   
   The reason I removed it as part of this PR, rather than as a separate issue, is because the execution of checkstyle is part of the CI workflow for ZooKeeper, which was the focus of this PR.
   
   Normally, you would specify it similarly to:
   
   ```xml
   <build>
     <pluginManagement>
       <plugins>
         <plugin>
           <groupId>my.group</groupId>
           <artifactId>my.plugin</artifactId>
           <version />
           <!-- configuration for all executions of this plugin -->
           <configuration />
         </plugin>
       </plugins>
     </pluginManagement>
     <plugins>
       <plugin>
         <groupId>my.group</groupId>
         <artifactId>my.plugin</artifactId>
         <executions>
           <execution>
             <id />
             <goals />
             <!-- execution-specific configuration -->
             <configuration />
           </execution>
         </executions>
       </plugin>
     </plugins>
   </build>
   ```
   
   But we had:
   
   ```xml
   <build>
     <pluginManagement>
       <plugins>
         <plugin>
           <groupId>my.group</groupId>
           <artifactId>my.plugin</artifactId>
           <version />
           <executions />
         </plugin>
       </plugins>
     </pluginManagement>
     <!-- this section was missing, so the execution in the above was never activated
     <plugins>
       <plugin>
         <groupId>my.group</groupId>
         <artifactId>my.plugin</artifactId>
       </plugin>
     </plugins>
     -->
   </build>
   ```
   
   I hope that helps explain it.




----------------------------------------------------------------
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:
users@infra.apache.org