updated the configure.sh script to also install parity on MacOS #13
Conversation
configure.sh
Outdated
| if ! type parity > /dev/null; | ||
| then | ||
|
|
||
| platform='unknown' |
configure.sh
Outdated
| elif [[ "$unamestr" == 'Darwin' ]]; then | ||
| installMac; | ||
| else | ||
| echo "Platform: $unamestr unknown"; |
There was a problem hiding this comment.
I would wonder if it would make sense to have the if check for Mac and the default case be Linux/Ubuntu/Debian.
There was a problem hiding this comment.
ya that would make more sense, I can make those changes
configure.sh
Outdated
|
|
||
| platform='unknown' | ||
| unamestr=`uname` | ||
| if [[ "$unamestr" == 'Linux' ]]; then |
There was a problem hiding this comment.
Not sure it makes sense to check for "Linux" for apt-specific instructions. Or "Darwin" when it uses Homebrew which I don't think is guaranteed to exist on Mac.
Maybe branch based on the existence of an actual package manager like apt or brew?
There was a problem hiding this comment.
Oh I see, your script installs Homebrew. I'd still maybe favour this approach so that installing Homebrew isn't a side effect of this script when it might not be documented/expected. That line also relies on Ruby which may/may not be installed either.
There was a problem hiding this comment.
ruby is default installed on macOS. I tried using the mac version of the linux install script to install parity from the website, but that does not install the commandline tool to start parity. If you are aware of a better way that I could install it then I can switch it to that but this does the job. Also I can add it to the README that it will install homebrew.
There was a problem hiding this comment.
Ah okay, ruby being definitely installed helps.
At any rate, my thought was more about instead of switching based on OS, switch based on existence of a package manager. So if brew is installed call a function that uses brew. If apt is installed call one that uses apt. If pacman or yum or whatever is installed eventually we can use that.
Just decouples package manager logic from operating system logic and avoids side-effects in one fell swoop.
There was a problem hiding this comment.
Thank make sense I can make those changes.
configure.sh
Outdated
| echo "Parity is already installed." | ||
| fi | ||
|
|
||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Random whitespace at the bottom here probably doesn't have to be in here. There are also newlines in the functions that I find make it a little trickier to read (e.g. 56-57, 48).
Update the script as well as the readme to reflect the change in the script to work on mac.