Github user shaneknapp commented on a diff in the pull request:

    https://github.com/apache/spark/pull/22854#discussion_r228584294
  
    --- Diff: build/mvn ---
    @@ -163,8 +163,19 @@ export MAVEN_OPTS=${MAVEN_OPTS:-"$_COMPILE_JVM_OPTS"}
     
     echo "Using \`mvn\` from path: $MVN_BIN" 1>&2
     
    -# Last, call the `mvn` command as usual
    +# call the `mvn` command as usual
     "${MVN_BIN}" -DzincPort=${ZINC_PORT} "$@"
     
    -# Try to shut down zinc explicitly
    -"${ZINC_BIN}" -shutdown -port ${ZINC_PORT}
    +# check to see if zinc server is still running post-build
    +"${ZINC_BIN}" -status -port ${ZINC_PORT} &> /dev/null
    +ZINC_STATUS=$?
    +
    +# Try to shut down zinc explicitly if the server is still running
    +if [ $ZINC_STATUS -eq 0 ]; then
    +  # zinc is still running!
    +  "${ZINC_BIN}" -shutdown -port ${ZINC_PORT}
    +  exit 0
    --- End diff --
    
    i put the else in there for clarity, and in case we want to ever do 
something (like report that zinc timed out etc) if the exit code on the status 
is 1.
    
    ¯\_(ツ)_/¯
    
    i'm also not a fan of putting `exit 0` at the end of any bash script, 
anywhere, ever.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to