[GitHub] [hadoop] amahussein edited a comment on pull request #3503: HADOOP-17952. Replace Guava VisibleForTesting by Hadoop's own annotation in hadoop-common-project modules

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

amahussein edited a comment on pull request #3503:
URL: https://github.com/apache/hadoop/pull/3503#issuecomment-934613772


   
   
   
   > @amahussein We are already using this dependency in parent pom. Also, even though checkstyle rule fails, we can't prevent someone from using this import. With this dependency, the build will fail if someone uses this dependency in hadoop-common modules. The eventual goal is to remove project level dependencies and put it only in parent pom as part of final PR.
   > This is what we have followed in previous commits as well. For instance, #3087 and #3049
   
   Oh! I haven't seen those changes. It was confusing to see that illegal imports are still used in checkstyle.xml .
   ```xml
           <module name="IllegalImport">
             <property name="regexp" value="true"/>
             <property name="illegalPkgs" value="sun, com\.google\.common"/>
             <property name="illegalClasses" value="^org\.apache\.hadoop\.thirdparty\.com\.google\.common\.io\.BaseEncoding, ^org\.apache\.hadoop\.thirdparty\.com\.google\.common\.base\.(Optional|Function|Predicate|Supplier), ^org\.apache\.hadoop\.thirdparty\.com\.google\.common\.collect\.(ImmutableListMultimap)"/>
           </module>
           <module name="RedundantImport"/>
           <module name="UnusedImports"/>
   ```
   It would be good to have a jira to move the above guava classes to the bannedImports if it is not already there.
   
   The checkstyle was doing fine detecting those packages at least for the current phase when we are in the process of defining alternatives.
   
   The problem with those pom changes being pushed now is that the guava changes becomes sort of invasive making changes to pom files of each module. Also, since guava replacement commits take time to get merged, we would have individual modules defining their own import rules independently for quite some time.
   On the other hand,Pushing those changes after a major milestone would mean only one Jira and one PR (i.e., _HADOOP-XYZ: migrating banned imports to use restrict-imports-enforcer-rule_).
   
   
   


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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org