Script for Student "Health Check"#522
Conversation
| then | ||
| echo "Node is installed, skipping..."; | ||
| else | ||
| echo "Node is not installed."; |
There was a problem hiding this comment.
Would students need to restart their terminal to see that node is infact installed? I know that sometimes that needs to happen. Maybe add a line saying "If you've installed node but this script is still saying you haven't, please restart your terminal and run the script again"
There was a problem hiding this comment.
We should put that into the output at the very beginning
|
@emsesc had a first run on Linux and Windows (via Git bash). There are some issues, I will enter them as review comments tomorrow. |
| echo "--------------------------------"; | ||
| echo "Checking if node is installed..."; | ||
| echo "--------------------------------"; | ||
| if which node > /dev/null |
There was a problem hiding this comment.
This leads to a very weird output on git bash on Windows
. I think we can transform this check into the version check done above => if the variable is empty, no Node.js is installed
| then | ||
| echo "Node is installed, skipping..."; | ||
| else | ||
| echo "Node is not installed."; |
There was a problem hiding this comment.
We should put that into the output at the very beginning
| if [[ (${nodev:1:2} == "16" || ${nodev:1:2} == "14") && (${funcv:0:1} == "4") ]] | ||
| then | ||
| echo "Compatible versions! You are good to go."; | ||
| elif [[ (${nodev:1:2} == "14" || ${nodev:1:2} == "12" || ${nodev:1:2} == "10") && (${funcv:0:1} == "3") ]] |
There was a problem hiding this comment.
Node.js version 10 and 12 are out of maintenance. They should be excluded; maybe a message might make sense when checking the Node version that those are not supported
|
Thanks for the suggestions @lechnerc77 and @ganning127 ! I've implemented them all. |
| echo "--------------------------------"; | ||
| echo "Checking if node is installed..."; | ||
| echo "--------------------------------"; | ||
| if [[ $(node -v) ]] |
There was a problem hiding this comment.
I think we can use ${nodev} and ${funcv} in the IF-clauses instead of calling node-v again.
Same on line 34
There was a problem hiding this comment.
@emsesc Overall looks really good now - also works in Git Bash as expected
lechnerc77
left a comment
lechnerc77
left a comment
There was a problem hiding this comment.
One small addition to get rid of another command
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
3 participants
Changes made
@lechnerc77 I believe this script is compatible with MacOS, Windows, and Linux. Students can run the bash script in Git Bash since they'll have that installed in order to work with Git if they have a Windows system.
Closes #521