keith-turner commented on a change in pull request #254:
URL: https://github.com/apache/fluo-uno/pull/254#discussion_r476907831
##########
File path: bin/impl/fetch.sh
##########
@@ -122,13 +116,20 @@ zookeeper)
fetch_zookeeper
;;
*)
- echo "Usage: uno fetch <component>"
- echo -e "\nPossible components:\n"
- echo " accumulo Downloads Accumulo, Hadoop & ZooKeeper. Builds Accumulo
if repo set in uno.conf"
- echo " fluo Downloads Fluo, Accumulo, Hadoop & ZooKeeper. Builds
Fluo or Accumulo if repo set in uno.conf"
- echo " hadoop Downloads Hadoop"
- echo " zookeeper Downloads ZooKeeper"
- echo "Options:"
- echo " --no-deps Dependencies will be fetched unless this option is
specified. Only works for fluo & accumulo components."
+ cat <<EOF
+Usage: uno fetch <component>
+
+Possible components:
+
+ accumulo Downloads Accumulo, Hadoop & ZooKeeper. Builds Accumulo if repo
set in uno.conf
+ fluo Downloads Fluo, Accumulo, Hadoop & ZooKeeper. Builds Fluo or
Accumulo if repo set in uno.conf
+ hadoop Downloads Hadoop
+ zookeeper Downloads ZooKeeper
+
+Options:
+ --no-deps Dependencies will be fetched unless this option is specified.
Only works for fluo & accumulo components.
+EOF
exit 1
esac
+
+# fetch.sh
Review comment:
What is the purpose of these comments?
##########
File path: bin/uno
##########
@@ -17,89 +17,33 @@
# Start: Resolve Script Directory
SOURCE="${BASH_SOURCE[0]}"
-while [[ -h "$SOURCE" ]]; do # resolve $SOURCE until the file is no longer a
symlink
+# resolve $SOURCE until the file is no longer a symlink
+while [[ -h "$SOURCE" ]]; do
bin="$( cd -P "$( dirname "$SOURCE" )" && pwd )"
SOURCE="$(readlink "$SOURCE")"
- [[ $SOURCE != /* ]] && SOURCE="$bin/$SOURCE" # if $SOURCE was a relative
symlink, we need to resolve it relative to the path where the symlink file was
located
+ # if $SOURCE was a relative symlink, we need to resolve it relative to the
+ # path where the symlink file was located
+ [[ $SOURCE != /* ]] && SOURCE="$bin/$SOURCE"
done
bin="$( cd -P "$( dirname "$SOURCE" )" && pwd )"
# Stop: Resolve Script Directory
-source "$bin"/impl/load-env.sh "$1"
-source "$UNO_HOME"/bin/impl/util.sh
+uno_cmd=$1
+shift
-case "$1" in
- ashell)
- check_dirs ACCUMULO_HOME
- "$ACCUMULO_HOME"/bin/accumulo shell -u "$ACCUMULO_USER" -p
"$ACCUMULO_PASSWORD" "${@:2}"
- ;;
- zk)
- check_dirs ZOOKEEPER_HOME
- "$ZOOKEEPER_HOME"/bin/zkCli.sh
- ;;
- env)
- "$bin"/impl/print-env.sh "${@:2}"
- ;;
- fetch)
- hash mvn 2>/dev/null || { echo >&2 "Maven must be installed & on PATH.
Aborting."; exit 1; }
- hash wget 2>/dev/null || { echo >&2 "wget must be installed & on PATH.
Aborting."; exit 1; }
- if [[ "$2" == "all" ]]; then
- "$bin"/impl/fetch.sh fluo
- else
- "$bin"/impl/fetch.sh "$2" "$3"
- fi
- ;;
- install)
- "$bin"/impl/install.sh "${@:2}"
- ;;
- kill)
- "$bin"/impl/kill.sh "${@:2}"
- ;;
- run)
- "$bin"/impl/run.sh "${@:2}"
- ;;
- setup)
- "$bin"/impl/setup.sh "${@:2}"
- ;;
- start)
- "$bin"/impl/start.sh "${@:2}"
- ;;
- stop)
- "$bin"/impl/stop.sh "${@:2}"
- ;;
- version)
- "$bin"/impl/version.sh "${@:2}"
- ;;
-
- status)
- "$bin"/impl/status.sh "${@:2}"
- ;;
-
- wipe)
- "$bin"/impl/kill.sh
- if [[ -d "$INSTALL" ]]; then
- echo "removing $INSTALL"
- rm -rf "$INSTALL"
- fi
+# shellcheck source=bin/impl/load-env.sh
+source "$bin"/impl/load-env.sh "$uno_cmd"
+# shellcheck source=bin/impl/commands.sh
+source "$UNO_HOME"/bin/impl/commands.sh
+
+case "$uno_cmd" in
+ ashell|env|fetch|install|kill|run|setup|start|status|stop|version|wipe|zk)
+ "uno_${uno_cmd}_main" "$@"
Review comment:
This is a nice refactoring. Its a bit hard to review, I am just going
to trust that you copied the code from the files to functions correctly.
Instead of having the switch statement enumerate all the command names,
wonder if it would work to check if the function exists instead. Maybe
something like the following, although I am not sure if it would even work.
```bash
if [ "$(type -t uno_${uno_cmd}_main)" = 'function' ]; then
"uno_${uno_cmd}_main" "$@"
else
uno_help_main "$@"
fi
```
Below is a post I found about checking if a function exists in bash to write
the above.
https://stackoverflow.com/questions/1007538/check-if-a-function-exists-from-a-bash-script
----------------------------------------------------------------
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]
