[GitHub] [orc] zabetak commented on a diff in pull request #1403: ORC-1371: Remove unsupported SLF4J bindings from classpath

["zabetak (via GitHub)" <gi...@apache.org>]

zabetak commented on code in PR #1403:
URL: https://github.com/apache/orc/pull/1403#discussion_r1101212637


##########
java/mapreduce/pom.xml:
##########
@@ -69,7 +69,6 @@
     <dependency>
       <groupId>org.apache.hadoop</groupId>
       <artifactId>hadoop-mapreduce-client-jobclient</artifactId>
-      <version>${min.hadoop.version}</version>

Review Comment:
   We need to add an exclusion for `hadoop-mapreduce-client-jobclient`. This can be done here or in the `dependencyManagement` section in the parent `pom.xml`.
   
   If we do it here then if someone in the future adds `hadoop-mapreduce-client-jobclient` in another module without copying the exclusion things will be broken again. The same can happen if `hadoop-mapreduce-client-jobclient` appears transitively from somewhere else. 
   
   On the other hand, if we add `hadoop-mapreduce-client-jobclient` in the `dependencyManagement` we are sure that the problems mentioned previously will not happen.
   
   In general it is a good practice to add all dependencies in the [dependency-management section|https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#dependency-management] so I am always in favor of doing that.
   
   Opinions can differ though, so I am happy to make the changes if you think that it would be better to just add the exclusion here.



-- 
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: issues-unsubscribe@orc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org