[GitHub] [hbase] apurtell commented on a change in pull request #2714: HBASE-25333 Add a yetus check to disable usage of VisibleForTesting a…

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

apurtell commented on a change in pull request #2714:
URL: https://github.com/apache/hbase/pull/2714#discussion_r536272527



##########
File path: dev-support/hbase-personality.sh
##########
@@ -816,6 +816,69 @@ function hbaseanti_patchfile
   return 0
 }
 
+######################################
+
+add_test_type visible-for-testing
+
+## @description  visible-for-testing file filter
+## @audience     private
+## @stability    evolving
+## @param        filename
+function visible-for-testing_filefilter
+{
+  local filename=$1
+
+  if [[ ${filename} =~ \.java$ ]]; then
+    add_test visible-for-testing
+  fi
+}
+
+## @description  visible-for-testing patch file check
+## @audience     private
+## @stability    evolving
+## @param        repostatus
+function visible-for-testing_precompile
+{
+  local repostatus=$1
+  local logfile="${PATCH_DIR}/patch-visible-for-testing.txt"
+  local errors
+
+  if [[ "${repostatus}" = branch ]]; then
+    return 0
+  fi
+
+  if ! verify_needed_test visible-for-testing; then
+    return 0
+  fi
+
+  big_console_header "VisibleForTesting plugin: ${BUILDMODE}"
+  if [[ "${BUILDMODE}" == "full" ]]; then
+    yetus_debug "going to check the whole repo"
+    find -type f -name *.java | grep -v generated | xargs grep -l "@InterfaceAudience.Public\|@InterfaceAudience.LimitedPrivate" | xargs grep -l @VisibleForTesting >> ${logfile}

Review comment:
       I would prefer we reject VisibleForTesting globally. 
   
   What purpose does it serve? An annotation is useful when it supports tooling, like an IDE, or some static analysis (error-prone?). We don't have anything like that depending on VisibleForTesting. So in my view VisibleForTesting is pointless and, worse, brings in Guava, which has a history of problematic compatibility problems. What happens when Google decides to remove VisibleForTesting from Guava some day? Why should we set ourselves up for that risk. There must be some reason. I see no good reason. 




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