cyphernode icon indicating copy to clipboard operation
cyphernode copied to clipboard

review dist/setup.sh

Open darwin opened this issue 7 years ago • 0 comments

While investigating #83 I looked briefly at dist/setup.sh and I think it needs a review.

Issues spotted:

  • paths/arguments are not quoted consistently. For example usage of $current_path is all over the place used without quoting. Imagine someone clones your git repo onto a path with a space character in it, this will fall flat on its face.
  • usage of ansi colors should be abstracted into a function, also it should be optional and be disabled when terminal does not support colors or color support it disabled in non-tty sessions.

Suggestions:

  • current_path seems to be hardcoded to the dist directory, this should be configurable and I should be able to generate multiple independent cyphernodes outside the repo checkout
  • step/try/next is suspicious, I would rather see whole script running with set -e -o pipefail. Then you can always opt-out with set +e explicitly when doing something which is expected to exit with error status code which you want to handle "by hand". If you want to give user a feedback on error, you should trap ... ERR. When done properly this should trap all errors consistently, including those in pipes and outside your try wrapper.
  • the script leaves a bunch of untracked files in the repo checkout, this looks dirty:
git status
On branch dev
Your branch is up to date with 'origin/dev'.

Untracked files:
  (use "git add <file>..." to include in what will be committed)

	bitcoin/
	client.7z
	config.7z
	docker-compose.yaml
	gatekeeper/
	installer/
	lightning/
	start.sh
	stop.sh
	testfeatures.sh
	traefik/

nothing added to commit but untracked files present (use "git add" to track)
  • instead configuring individual paths to different service data folders, I would ask once for "data" directory and derive all paths from there by default, if user wants to go crazy, they can override those configurations by hand via env variables or tweaking created config.sh, as you will be adding more and more services/containers, this will become tedious...

darwin avatar Apr 08 '19 21:04 darwin