[GitHub] [hbase] taklwu commented on a change in pull request #2737: HBASE-25362 Fix quoting in hbase-vote to avoid voting build failures

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

taklwu commented on a change in pull request #2737:
URL: https://github.com/apache/hbase/pull/2737#discussion_r538695743



##########
File path: dev-support/hbase-vote.sh
##########
@@ -145,17 +148,17 @@ function unzip_from_source() {
 
 function rat_test() {
     rm -f "${OUTPUT_PATH_PREFIX}"_rat_test
-    mvn clean apache-rat:check "${MVN_PROPERTIES}" 2>&1 | tee "${OUTPUT_PATH_PREFIX}"_rat_test && RAT_CHECK_PASSED=1
+    mvn clean apache-rat:check "${MVN_PROPERTIES[@]}" 2>&1 | tee "${OUTPUT_PATH_PREFIX}"_rat_test && RAT_CHECK_PASSED=1
 }
 
 function build_from_source() {
     rm -f "${OUTPUT_PATH_PREFIX}"_build_from_source
-    mvn clean install "${MVN_PROPERTIES}" -DskipTests 2>&1 | tee "${OUTPUT_PATH_PREFIX}"_build_from_source && BUILD_FROM_SOURCE_PASSED=1
+    mvn clean install "${MVN_PROPERTIES[@]}" -DskipTests 2>&1 | tee "${OUTPUT_PATH_PREFIX}"_build_from_source && BUILD_FROM_SOURCE_PASSED=1

Review comment:
       [no action needed] put a note here that `-DskipTests` is intentionally overriding any input to skip the unit tests from the give properties such that `build_from_source` verify the build quickly

##########
File path: dev-support/hbase-vote.sh
##########
@@ -167,11 +170,11 @@ function print_when_exit() {
         * Signature: $( ((SIGNATURE_PASSED)) && echo "ok" || echo "failed" )
         * Checksum : $( ((CHECKSUM_PASSED)) && echo "ok" || echo "failed" )
         * Rat check (${JAVA_VERSION}): $( ((RAT_CHECK_PASSED)) && echo "ok" || echo "failed" )
-         - mvn clean apache-rat:check "${MVN_PROPERTIES}"
+         - mvn clean apache-rat:check ${MVN_PROPERTIES[@]}
         * Built from source (${JAVA_VERSION}): $( ((BUILD_FROM_SOURCE_PASSED)) && echo "ok" || echo "failed" )
-         - mvn clean install -DskipTests "${MVN_PROPERTIES}"
+         - mvn clean install -DskipTests ${MVN_PROPERTIES[@]}

Review comment:
       [optional] for accurate messaging, maybe moving `-DskipTests` to the end? 




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