ctubbsii commented on a change in pull request #1910:
URL: https://github.com/apache/accumulo/pull/1910#discussion_r583864338
##########
File path: assemble/bin/accumulo
##########
@@ -58,6 +58,18 @@ function main() {
echo "$CLASSPATH"
exit 0
fi
+
+ # Set up path variable for default import config file
+ export jShellPath="$conf/jshell-init.jsh"
+
Review comment:
Remove trailing whitespace on empty lines. Also, I don't think this
variable needs to be exported, since it's only ever used within this same
script, and not used by any subcommands. However, it is used inside a function
(called 'main'), so you can make it a local variable, so its assignment doesn't
leak outside the function. Annoyingly, the default in bash is that assignments
create global variables.
```suggestion
# Set up path variable for default import config file
local jShellPath="$conf/jshell-init.jsh"
```
##########
File path: assemble/src/main/scripts/create-jshell.sh
##########
@@ -0,0 +1,110 @@
+#! /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(){
+ export srcDir="$1"
+ export jPath="$2"
+ export client="accumulo/core/client"
+ export data="accumulo/core/data"
+ export security="accumulo/core/security"
+ export mini="accumulo/minicluster"
+ export hadoop="accumulo/hadoop"
+
+ # Does an auto-generated JShell config file exists?
+ if [[ ! -e "$jPath" ]]; then
+ echo "Cannot add APIs in $jPath"
+ echo "Please ensure jshell-init.jsh exists"
+ exit 1
+ fi
+
+ # Is the source directory valid?
+ if [[ ! -d "$srcDir" ]]; then
+ echo "$srcDir is not a valid directory. Please make sure it exists."
+ rm "$jPath"
+ exit 1
+ fi
+
+ # Does a valid JShell path and source directory exists?
+ if [[ -e "$jPath" ]] && [[ -d "$srcDir" ]]; then
+ # Add API category designator in jshell-init.jsh
+ case "$srcDir" in
+ *"$client"*) echo "// Accumulo Client API" >> "$jPath";;
+ *"$data"*) echo "// Accumulo Data API" >> "$jPath";;
+ *"$security"*) echo "// Accumulo Security API" >> "$jPath";;
+ *"$mini"*) echo "// Accumulo Minicluster API" >> "$jPath";;
+ *"$hadoop"*) echo "// Accumulo Hadoop API" >> "$jPath";;
+ *) echo "// Other API" >> "$jPath";;
+ esac
+
+ # Extract API info from provided source directory
+ mapfile -t api < <(find "$srcDir" -type f -name '*.java'|
+ xargs -n1 dirname| sort -u)
Review comment:
When I ran shellcheck on this script, it warned of files containing
special characters that might propose a problem. Making the output of `find` a
null `\0` character and the input of `xargs` look split on null characters, the
risk goes away:
```suggestion
# Extract API info from provided source directory
mapfile -t api < <(find "$srcDir" -type f -name '*.java' -print0 |
xargs -0 -n1 dirname| sort -u)
```
##########
File path: assemble/src/main/scripts/create-jshell.sh
##########
@@ -0,0 +1,110 @@
+#! /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(){
+ export srcDir="$1"
+ export jPath="$2"
+ export client="accumulo/core/client"
+ export data="accumulo/core/data"
+ export security="accumulo/core/security"
+ export mini="accumulo/minicluster"
+ export hadoop="accumulo/hadoop"
+
+ # Does an auto-generated JShell config file exists?
+ if [[ ! -e "$jPath" ]]; then
+ echo "Cannot add APIs in $jPath"
+ echo "Please ensure jshell-init.jsh exists"
+ exit 1
+ fi
+
+ # Is the source directory valid?
+ if [[ ! -d "$srcDir" ]]; then
+ echo "$srcDir is not a valid directory. Please make sure it exists."
+ rm "$jPath"
+ exit 1
+ fi
+
+ # Does a valid JShell path and source directory exists?
+ if [[ -e "$jPath" ]] && [[ -d "$srcDir" ]]; then
+ # Add API category designator in jshell-init.jsh
+ case "$srcDir" in
+ *"$client"*) echo "// Accumulo Client API" >> "$jPath";;
+ *"$data"*) echo "// Accumulo Data API" >> "$jPath";;
+ *"$security"*) echo "// Accumulo Security API" >> "$jPath";;
+ *"$mini"*) echo "// Accumulo Minicluster API" >> "$jPath";;
+ *"$hadoop"*) echo "// Accumulo Hadoop API" >> "$jPath";;
+ *) echo "// Other API" >> "$jPath";;
+ esac
+
+ # Extract API info from provided source directory
+ mapfile -t api < <(find "$srcDir" -type f -name '*.java'|
+ xargs -n1 dirname| sort -u)
+
+ # Load in API and format source directory into Java import statements
+ for apiPath in "${api[@]}"; do
+ printf "%s\n" "import ${apiPath##*/java/}.*" >> "$jPath"
+ done
+ sed -i '/^ *import / s#/#.#g' "$jPath"
+ echo " " >> "$jPath"
+ fi
+}
+
+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
+ export scriptPath="$( cd -P "$( dirname "${SOURCE}" )" && pwd )";
+ export mainBase=$( cd -P "${scriptPath}"/../../../.. && pwd );
+ export jPath="$mainBase/assemble/target/jshell-init.jsh"
+ export corePath="core/src/main/java/org/apache/accumulo/core"
+ export miniPath="minicluster/src/main/java/org/apache/accumulo"
+ export hadoopPath="hadoop-mapreduce/src/main/java/org/apache/accumulo"
+
+ # Create path to Accumulo Public API Source Directories
+ export CLIENT="$mainBase/$corePath/client"
+ export DATA="$mainBase/$corePath/data"
+ export SECURITY="$mainBase/$corePath/security"
+ export MINI="$mainBase/$miniPath/minicluster"
+ export HADOOP="$mainBase/$hadoopPath/hadoop/mapreduce"
Review comment:
shellcheck warns of return code suppression when assigning and exporting
at the same time. It would also warn after these are converted to local
variables, which can also be done, since these don't need to be exported. This
only applies when assigning a variable to the result of a command, but not to
simple assignments.
I strongly recommend running shellcheck to catch any potential errors on any
scripts you write.
```suggestion
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"
```
##########
File path: assemble/src/main/scripts/create-jshell.sh
##########
@@ -0,0 +1,110 @@
+#! /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(){
+ export srcDir="$1"
+ export jPath="$2"
+ export client="accumulo/core/client"
+ export data="accumulo/core/data"
+ export security="accumulo/core/security"
+ export mini="accumulo/minicluster"
+ export hadoop="accumulo/hadoop"
+
+ # Does an auto-generated JShell config file exists?
+ if [[ ! -e "$jPath" ]]; then
+ echo "Cannot add APIs in $jPath"
+ echo "Please ensure jshell-init.jsh exists"
+ exit 1
+ fi
+
+ # Is the source directory valid?
+ if [[ ! -d "$srcDir" ]]; then
+ echo "$srcDir is not a valid directory. Please make sure it exists."
+ rm "$jPath"
+ exit 1
+ fi
+
+ # Does a valid JShell path and source directory exists?
+ if [[ -e "$jPath" ]] && [[ -d "$srcDir" ]]; then
+ # Add API category designator in jshell-init.jsh
+ case "$srcDir" in
+ *"$client"*) echo "// Accumulo Client API" >> "$jPath";;
+ *"$data"*) echo "// Accumulo Data API" >> "$jPath";;
+ *"$security"*) echo "// Accumulo Security API" >> "$jPath";;
+ *"$mini"*) echo "// Accumulo Minicluster API" >> "$jPath";;
+ *"$hadoop"*) echo "// Accumulo Hadoop API" >> "$jPath";;
+ *) echo "// Other API" >> "$jPath";;
+ esac
+
+ # Extract API info from provided source directory
+ mapfile -t api < <(find "$srcDir" -type f -name '*.java'|
+ xargs -n1 dirname| sort -u)
+
+ # Load in API and format source directory into Java import statements
+ for apiPath in "${api[@]}"; do
+ printf "%s\n" "import ${apiPath##*/java/}.*" >> "$jPath"
+ done
+ sed -i '/^ *import / s#/#.#g' "$jPath"
+ echo " " >> "$jPath"
+ fi
+}
+
+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
+ export scriptPath="$( cd -P "$( dirname "${SOURCE}" )" && pwd )";
+ export mainBase=$( cd -P "${scriptPath}"/../../../.. && pwd );
+ export jPath="$mainBase/assemble/target/jshell-init.jsh"
+ export corePath="core/src/main/java/org/apache/accumulo/core"
+ export miniPath="minicluster/src/main/java/org/apache/accumulo"
+ export hadoopPath="hadoop-mapreduce/src/main/java/org/apache/accumulo"
+
+ # Create path to Accumulo Public API Source Directories
+ export CLIENT="$mainBase/$corePath/client"
+ export DATA="$mainBase/$corePath/data"
+ export SECURITY="$mainBase/$corePath/security"
+ export MINI="$mainBase/$miniPath/minicluster"
+ export HADOOP="$mainBase/$hadoopPath/hadoop/mapreduce"
+
+ # Does an auto-generated JShell config file exists?
+ if [[ -e "$jPath" ]]; then
+ rm "$jPath"
+ fi
+
+ # Create new jshell-init file and load in license header
+ touch "$jPath"
Review comment:
This file can be erased and created at the same time. Also, you can make
sure its directory is created also, in case the script runs before other parts
of Maven that normally create the target directory:
```suggestion
# Create new jshell-init file
mkdir -p "$mainBase/assemble/target"
:> "$jPath"
```
##########
File path: assemble/src/main/scripts/create-jshell.sh
##########
@@ -0,0 +1,110 @@
+#! /usr/bin/env bash
Review comment:
This file has a lot of lines that have trailing whitespace. It'd be good
to remove any trailing spaces on all lines in the file. If you're using VIM,
you can do this with the command:
```
:1,$ s/ *$//c
```
This goes through lines 1, through $ (marker for last line in file), and
replaces any single space followed by an optional sequence of multiple spaces
at the end of each line with an empty string, and the `c` at the end is to
confirm each one, so you can make sure it's working correctly. Drop the `c` if
you just want to run without checking.
##########
File path: assemble/src/main/scripts/create-jshell.sh
##########
@@ -0,0 +1,110 @@
+#! /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(){
+ export srcDir="$1"
+ export jPath="$2"
+ export client="accumulo/core/client"
+ export data="accumulo/core/data"
+ export security="accumulo/core/security"
+ export mini="accumulo/minicluster"
+ export hadoop="accumulo/hadoop"
Review comment:
I don't think any of the variables in this entire script need to be
exported, but they can all be declared as local variables. A few other
variables don't have any exports, but can also be declared as local variables
if they are inside a function.
##########
File path: assemble/src/main/scripts/create-jshell.sh
##########
@@ -0,0 +1,110 @@
+#! /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(){
+ export srcDir="$1"
+ export jPath="$2"
+ export client="accumulo/core/client"
+ export data="accumulo/core/data"
+ export security="accumulo/core/security"
+ export mini="accumulo/minicluster"
+ export hadoop="accumulo/hadoop"
+
+ # Does an auto-generated JShell config file exists?
+ if [[ ! -e "$jPath" ]]; then
+ echo "Cannot add APIs in $jPath"
+ echo "Please ensure jshell-init.jsh exists"
+ exit 1
+ fi
+
+ # Is the source directory valid?
+ if [[ ! -d "$srcDir" ]]; then
+ echo "$srcDir is not a valid directory. Please make sure it exists."
+ rm "$jPath"
+ exit 1
+ fi
+
+ # Does a valid JShell path and source directory exists?
+ if [[ -e "$jPath" ]] && [[ -d "$srcDir" ]]; then
+ # Add API category designator in jshell-init.jsh
+ case "$srcDir" in
+ *"$client"*) echo "// Accumulo Client API" >> "$jPath";;
+ *"$data"*) echo "// Accumulo Data API" >> "$jPath";;
+ *"$security"*) echo "// Accumulo Security API" >> "$jPath";;
+ *"$mini"*) echo "// Accumulo Minicluster API" >> "$jPath";;
+ *"$hadoop"*) echo "// Accumulo Hadoop API" >> "$jPath";;
+ *) echo "// Other API" >> "$jPath";;
+ esac
+
+ # Extract API info from provided source directory
+ mapfile -t api < <(find "$srcDir" -type f -name '*.java'|
+ xargs -n1 dirname| sort -u)
+
+ # Load in API and format source directory into Java import statements
+ for apiPath in "${api[@]}"; do
+ printf "%s\n" "import ${apiPath##*/java/}.*" >> "$jPath"
+ done
+ sed -i '/^ *import / s#/#.#g' "$jPath"
+ echo " " >> "$jPath"
Review comment:
You could just use echo here instead of printf, since the only thing
printf is doing is adding the newline, which echo would do by default. `tr` is
probably also faster than `sed`, and the extra blank line doesn't need a space:
```suggestion
echo "import ${apiPath##*/java/}.*" | tr / . >> "$jPath"
done
echo >> "$jPath"
```
##########
File path: assemble/src/main/scripts/create-jshell.sh
##########
@@ -0,0 +1,110 @@
+#! /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(){
+ export srcDir="$1"
+ export jPath="$2"
+ export client="accumulo/core/client"
+ export data="accumulo/core/data"
+ export security="accumulo/core/security"
+ export mini="accumulo/minicluster"
+ export hadoop="accumulo/hadoop"
+
+ # Does an auto-generated JShell config file exists?
+ if [[ ! -e "$jPath" ]]; then
+ echo "Cannot add APIs in $jPath"
+ echo "Please ensure jshell-init.jsh exists"
+ exit 1
+ fi
+
+ # Is the source directory valid?
+ if [[ ! -d "$srcDir" ]]; then
+ echo "$srcDir is not a valid directory. Please make sure it exists."
+ rm "$jPath"
+ exit 1
+ fi
+
+ # Does a valid JShell path and source directory exists?
+ if [[ -e "$jPath" ]] && [[ -d "$srcDir" ]]; then
+ # Add API category designator in jshell-init.jsh
+ case "$srcDir" in
+ *"$client"*) echo "// Accumulo Client API" >> "$jPath";;
+ *"$data"*) echo "// Accumulo Data API" >> "$jPath";;
+ *"$security"*) echo "// Accumulo Security API" >> "$jPath";;
+ *"$mini"*) echo "// Accumulo Minicluster API" >> "$jPath";;
+ *"$hadoop"*) echo "// Accumulo Hadoop API" >> "$jPath";;
+ *) echo "// Other API" >> "$jPath";;
+ esac
+
+ # Extract API info from provided source directory
+ mapfile -t api < <(find "$srcDir" -type f -name '*.java'|
+ xargs -n1 dirname| sort -u)
+
+ # Load in API and format source directory into Java import statements
+ for apiPath in "${api[@]}"; do
+ printf "%s\n" "import ${apiPath##*/java/}.*" >> "$jPath"
+ done
+ sed -i '/^ *import / s#/#.#g' "$jPath"
+ echo " " >> "$jPath"
+ fi
+}
+
+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
+ export scriptPath="$( cd -P "$( dirname "${SOURCE}" )" && pwd )";
+ export mainBase=$( cd -P "${scriptPath}"/../../../.. && pwd );
+ export jPath="$mainBase/assemble/target/jshell-init.jsh"
+ export corePath="core/src/main/java/org/apache/accumulo/core"
+ export miniPath="minicluster/src/main/java/org/apache/accumulo"
+ export hadoopPath="hadoop-mapreduce/src/main/java/org/apache/accumulo"
+
+ # Create path to Accumulo Public API Source Directories
+ export CLIENT="$mainBase/$corePath/client"
+ export DATA="$mainBase/$corePath/data"
+ export SECURITY="$mainBase/$corePath/security"
+ export MINI="$mainBase/$miniPath/minicluster"
+ export HADOOP="$mainBase/$hadoopPath/hadoop/mapreduce"
+
+ # Does an auto-generated JShell config file exists?
+ if [[ -e "$jPath" ]]; then
+ rm "$jPath"
+ fi
+
+ # Create new jshell-init file and load in license header
+ touch "$jPath"
+
+ # Create and add Accumulo APIs into API storage
+ apiStorage=("$CLIENT" "$DATA" "$SECURITY" "$MINI" "$HADOOP")
+
+ # Traverse through each source directory and load in Accumulo APIs
+ for srcDir in "${apiStorage[@]}"; do
Review comment:
These local variables, like arrays and loop variables are easy to
overlook and accidentally create global variables. Global variables aren't
necessarily a problem in a small single-threaded script, but it's good practice
to keep variables as local as possible.
```suggestion
local apiStorage=("$CLIENT" "$DATA" "$SECURITY" "$MINI" "$HADOOP")
# Traverse through each source directory and load in Accumulo APIs
local srcDir
for srcDir in "${apiStorage[@]}"; do
```
##########
File path: assemble/src/main/scripts/create-jshell.sh
##########
@@ -0,0 +1,110 @@
+#! /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(){
+ export srcDir="$1"
+ export jPath="$2"
+ export client="accumulo/core/client"
+ export data="accumulo/core/data"
+ export security="accumulo/core/security"
+ export mini="accumulo/minicluster"
+ export hadoop="accumulo/hadoop"
+
+ # Does an auto-generated JShell config file exists?
+ if [[ ! -e "$jPath" ]]; then
+ echo "Cannot add APIs in $jPath"
+ echo "Please ensure jshell-init.jsh exists"
+ exit 1
+ fi
+
+ # Is the source directory valid?
+ if [[ ! -d "$srcDir" ]]; then
+ echo "$srcDir is not a valid directory. Please make sure it exists."
+ rm "$jPath"
+ exit 1
+ fi
+
+ # Does a valid JShell path and source directory exists?
+ if [[ -e "$jPath" ]] && [[ -d "$srcDir" ]]; then
+ # Add API category designator in jshell-init.jsh
+ case "$srcDir" in
+ *"$client"*) echo "// Accumulo Client API" >> "$jPath";;
+ *"$data"*) echo "// Accumulo Data API" >> "$jPath";;
+ *"$security"*) echo "// Accumulo Security API" >> "$jPath";;
+ *"$mini"*) echo "// Accumulo Minicluster API" >> "$jPath";;
+ *"$hadoop"*) echo "// Accumulo Hadoop API" >> "$jPath";;
+ *) echo "// Other API" >> "$jPath";;
+ esac
+
+ # Extract API info from provided source directory
+ mapfile -t api < <(find "$srcDir" -type f -name '*.java'|
+ xargs -n1 dirname| sort -u)
+
+ # Load in API and format source directory into Java import statements
+ for apiPath in "${api[@]}"; do
+ printf "%s\n" "import ${apiPath##*/java/}.*" >> "$jPath"
+ done
+ sed -i '/^ *import / s#/#.#g' "$jPath"
+ echo " " >> "$jPath"
+ fi
+}
+
+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
+ export scriptPath="$( cd -P "$( dirname "${SOURCE}" )" && pwd )";
+ export mainBase=$( cd -P "${scriptPath}"/../../../.. && pwd );
+ export jPath="$mainBase/assemble/target/jshell-init.jsh"
+ export corePath="core/src/main/java/org/apache/accumulo/core"
+ export miniPath="minicluster/src/main/java/org/apache/accumulo"
+ export hadoopPath="hadoop-mapreduce/src/main/java/org/apache/accumulo"
+
+ # Create path to Accumulo Public API Source Directories
+ export CLIENT="$mainBase/$corePath/client"
+ export DATA="$mainBase/$corePath/data"
+ export SECURITY="$mainBase/$corePath/security"
+ export MINI="$mainBase/$miniPath/minicluster"
+ export HADOOP="$mainBase/$hadoopPath/hadoop/mapreduce"
+
+ # Does an auto-generated JShell config file exists?
+ if [[ -e "$jPath" ]]; then
+ rm "$jPath"
+ fi
+
+ # Create new jshell-init file and load in license header
+ touch "$jPath"
+
+ # Create and add Accumulo APIs into API storage
+ apiStorage=("$CLIENT" "$DATA" "$SECURITY" "$MINI" "$HADOOP")
+
+ # Traverse through each source directory and load in Accumulo APIs
+ for srcDir in "${apiStorage[@]}"; do
+ addAccumuloAPI "$srcDir" "$jPath"
+ done
+ exit 0
Review comment:
Normally at the end of a function, you'd want to let the return code of
the last command propagate outside of the function. If this loop terminates
abnormally, we shouldn't mask its return code with our own using `exit 0`,
which forces it to terminate successfully, even if there was an error.
Also, I was thinking, you don't need to pass in the filename to the function
or create it ahead of time. Instead, you can just create the file right here at
the output of the whole for loop. That would simplify the implementation of the
addAccumuloAPI command a lot.
```suggestion
addAccumuloAPI "$srcDir"
done > "$jPath"
```
One other suggestion I had was that you could move the printing of the
section headers to outside that method, that way you don't need to create so
many variables in that method to do the section header checks. You could either
move the if statements to here, inside this for loop, before calling
`addAccumuloAPI`, or you remove the loop, and do something like the following.
You wouldn't even need all the extra variables this way, because you could just
inline them:
```bash
{
echo "// section header here for client"
addAccumuloAPI "$CLIENT"
echo "// section header here for data"
addAccumuloAPI "$DATA"
# etc...
} > "$jPath"
```
----------------------------------------------------------------
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]
