Issue #1691 - Improves installation script #1692
Conversation
… message if necessary
initial_setup.sh
Outdated
| if which yarn && [[ `yarn --version` == $YARN_VERSION ]]; then | ||
| echo 'Using pre-installed global Yarn'; YARN=yarn | ||
| else | ||
| if [ -f "./node_modules/.bin/yarn" ] && [[ `./node_modules/.bin/yarn --version` == $YARN_VERSION ]]; then |
There was a problem hiding this comment.
What happens if the script is not run from the correct directory?
There was a problem hiding this comment.
go to the root repository
It is mentioned in the instructions for README.md
We could have error handling for that but seems a bit far-fetched idea to me.
There was a problem hiding this comment.
Good point, maybe add a quick comment in the file itself to keep this assumption close to the source code
initial_setup.sh
Outdated
| if ! "$YARN" install; then | ||
| echo "Sorry, there seems to be something wrong, please correct the errors and try again" | ||
| echo "There was some error installing the required packages, please rectify the errors and try again" | ||
| exit 1 |
There was a problem hiding this comment.
We should be a bit more descriptive with our error message. Maybe something along the lines of "Unable to run yarn install successfully. You might not be connected to the internet, or there might be a problem with your FROG folder"
There was a problem hiding this comment.
That would be displayed by yarn itself. This just informs the user that his action wasn't successful.
initial_setup.sh
Outdated
| exit 0 | ||
| echo | ||
| echo -e "\xE2\x9C\xA8 Finished Initial Setup." | ||
| echo -e "\xF0\x9F\xA4\x93 Run ${LBLUE}npm start server ${NC}to begin hacking!" |
There was a problem hiding this comment.
nit: We could move the emojis to constants to help with readability
There was a problem hiding this comment.
We could introduce a flag to remove emoji.
There was a problem hiding this comment.
Unclear if there is benefit in doing so, this file will normally only be run once
Co-Authored-By: Quentin Golsteyn <[email protected]>
As described in #1691 the PR includes some changes which allow for checking exit code before presenting the success message.