ctubbsii commented on a change in pull request #1910: URL: https://github.com/apache/accumulo/pull/1910#discussion_r585866749
########## File path: assemble/src/main/scripts/create-jshell.sh ########## @@ -0,0 +1,92 @@ +#! /usr/bin/env bash +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +function addAccumuloAPI(){ + local srcDir="$1" + # Extract API info from provided source directory + mapfile -t api < <(find "$srcDir" -type f -name '*.java' -print0| + xargs -0 -n1 dirname| sort -u) + + # Load in API and format source directory into Java import statements + for apiPath in "${api[@]}"; do + echo "import ${apiPath##*/java/}.*" | tr / . + done + echo +} + +function main(){ + # Establish Accumulo's main base directory + SOURCE="${BASH_SOURCE[0]}" + while [[ -h "${SOURCE}" ]]; do + bin="$( cd -P "$( dirname "${SOURCE}" )" && pwd )" + SOURCE="$(readlink "${SOURCE}")" + [[ "${SOURCE}" != /* ]] && SOURCE="${bin}/${SOURCE}" + done + + # Establish file and folder paths for JShell config + local scriptPath + scriptPath="$( cd -P "$( dirname "${SOURCE}" )" && pwd )" + local mainBase + mainBase="$( cd -P "${scriptPath}"/../../../.. && pwd )" + local jPath="$mainBase/assemble/target/jshell-init.jsh" + local corePath="core/src/main/java/org/apache/accumulo/core" + local miniPath="minicluster/src/main/java/org/apache/accumulo" + local hadoopPath="hadoop-mapreduce/src/main/java/org/apache/accumulo" + + # Create path to Accumulo Public API Source Directories + local CLIENT="$mainBase/$corePath/client" + local DATA="$mainBase/$corePath/data" + local SECURITY="$mainBase/$corePath/security" + local MINI="$mainBase/$miniPath/minicluster" + local HADOOP="$mainBase/$hadoopPath/hadoop/mapreduce" + + # Create new jshell-init file + mkdir -p "$mainBase/assemble/target" + :> "$jPath" + + # Create and add Accumulo APIs into API storage + local apiStorage=("$CLIENT" "$DATA" "$SECURITY" "$MINI" "$HADOOP") + local srcDir + + # Validate each source directory before populating JShell-Init file + for srcDir in "${apiStorage[@]}"; do + if [[ ! -d "$srcDir" ]]; then + echo "Could not auto-generate jshell-init.jsh" + echo "$srcDir is not a valid directory. Please make sure it exists." + rm "$jPath" + exit 1 + fi + done + echo "Generating JShell-Init file" + { + echo "System.out.println(\"Preparing JShell for Apache Accumulo\")" + echo "// Accumulo Client API" + addAccumuloAPI "${apiStorage[0]}" + echo "// Accumulo Data API" + addAccumuloAPI "${apiStorage[1]}" + echo "// Accumulo Security API" + addAccumuloAPI "${apiStorage[2]}" + echo "// Accumulo MiniCluster API" + addAccumuloAPI "${apiStorage[3]}" + echo "// Accumulo Hadoop API" + addAccumuloAPI "${apiStorage[4]}" + } > "$jPath" +} +main "$@" Review comment: I had a few other minor tips for this script, but probably not worth going back and forth over them, so here they are as one lump suggestion: * Minor style improvements (indentation using 2 spaces instead of 3, space before opening brace in function declaration and around pipes * Declare additional variables as local (SOURCE, bin) and place in single line at top of each function * Do validation of srcDirs in called method, rather than in a separate loop, to avoid needing to use an array for the srcDirs * Reduce number of variables needed by inlining several * Remove redundant clearing of destination file * Use single quote for strings that don't have embedded variables * Move System.out.println to after the imports ```suggestion function addAccumuloAPI() { local srcDir="$1" api apiPath # Validate each source directory before populating JShell-Init file if [[ ! -d "$srcDir" ]]; then echo "$srcDir is not a valid directory. Please make sure it exists." exit 1 fi # Extract API info from provided source directory mapfile -t api < <(find "$srcDir" -type f -name '*.java' -print0 | xargs -0 -n1 dirname | sort -u) # Load in API and format source directory into Java import statements for apiPath in "${api[@]}"; do echo "import ${apiPath##*/java/}.*" | tr / . done echo } function main() { local SOURCE bin scriptPath mainBase corePath # Establish Accumulo's main base directory SOURCE="${BASH_SOURCE[0]}" while [[ -h "${SOURCE}" ]]; do bin="$( cd -P "$( dirname "${SOURCE}" )" && pwd )" SOURCE="$(readlink "${SOURCE}")" [[ "${SOURCE}" != /* ]] && SOURCE="${bin}/${SOURCE}" done # Establish file and folder paths for JShell config scriptPath="$( cd -P "$( dirname "${SOURCE}" )" && pwd )" mainBase="$( cd -P "${scriptPath}"/../../../.. && pwd )" corePath="$mainBase/core/src/main/java/org/apache/accumulo/core" # Create new jshell-init file mkdir -p "$mainBase/assemble/target" echo 'Generating JShell-Init file' { echo '// Accumulo Client API' addAccumuloAPI "$corePath/client" echo '// Accumulo Data API' addAccumuloAPI "$corePath/data" echo '// Accumulo Security API' addAccumuloAPI "$corePath/security" echo '// Accumulo MiniCluster API' addAccumuloAPI "$mainBase/minicluster/src/main/java/org/apache/accumulo/minicluster" echo '// Accumulo Hadoop API' addAccumuloAPI "$mainBase/hadoop-mapreduce/src/main/java/org/apache/accumulo/hadoop/mapreduce" echo echo 'System.out.println("Preparing JShell for Apache Accumulo")' echo } > "$mainBase/assemble/target/jshell-init.jsh" } main "$@" ``` ---------------------------------------------------------------- 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: [email protected]
