adoroszlai commented on a change in pull request #924:
URL: https://github.com/apache/hadoop-ozone/pull/924#discussion_r426490028



##########
File path: hadoop-hdds/common/pom.xml
##########
@@ -202,6 +202,20 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd";>
       </extension>
     </extensions>
     <plugins>
+      <plugin>
+        <groupId>com.salesforce.servicelibs</groupId>
+        <artifactId>proto-backwards-compatibility</artifactId>
+        <configuration>
+          <protoSourceRoot>${basedir}/src/main/proto/</protoSourceRoot>
+        </configuration>
+        <executions>
+          <execution>
+            <goals>
+              <goal>backwards-compatibility-check</goal>
+            </goals>
+          </execution>
+        </executions>

Review comment:
       Can we avoid multiplication of this config by adding it to the root pom 
instead?

##########
File path: hadoop-hdds/container-service/pom.xml
##########
@@ -100,6 +100,20 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd";>
 
   <build>
     <plugins>
+      <plugin>
+        <groupId>com.salesforce.servicelibs</groupId>
+        <artifactId>proto-backwards-compatibility</artifactId>

Review comment:
       The plugin's doc says [`os-maven-plugin` is required for it to 
work](https://github.com/salesforce/proto-backwards-compat-maven-plugin#usage), 
but it's missing from `hadoop-hdds/container-service/pom.xml` and 
`hadoop-ozone/common/pom.xml` (only present in the other two submodule pom 
files).

##########
File path: pom.xml
##########
@@ -241,6 +242,11 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xs
 
   <dependencyManagement>
     <dependencies>
+      <dependency>
+        <groupId>com.salesforce.servicelibs</groupId>
+        <artifactId>proto-backwards-compatibility</artifactId>
+        <version>${proto-backwards-compatibility.version}</version>

Review comment:
       I think this should be added to/as `<pluginManagement>/<plugin>` instead 
of `<dependencyManagement>/<dependency>`.  Currently Maven gives warning due to 
missing version:
   
   ```
   [WARNING] Some problems were encountered while building the effective model 
for org.apache.hadoop:hadoop-hdds-common:jar:0.6.0-SNAPSHOT
   [WARNING] 'build.plugins.plugin.version' for 
com.salesforce.servicelibs:proto-backwards-compatibility is missing. @ line 
205, column 15
   ```




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



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to