Node, Yarn and Docker version check added in dev.sh file #602
Conversation
dev.sh
Outdated
| exit 1 | ||
| fi | ||
|
|
||
| echo "Node.js, Yarn, and Docker versions are compatible. Proceeding..." |
There was a problem hiding this comment.
I don't think this needs to be printed. In the "happy path" we should show as little noise as possible.
There was a problem hiding this comment.
then i can remove this fully and show nothing
|
Please add a screenshot or screen recording of the change in action in the PR description |
dev.sh
Outdated
| @@ -1,5 +1,59 @@ | |||
| #!/bin/bash | |||
|
|
|||
| REQUIRED_NODE_VERSION=22.0.0 | |||
There was a problem hiding this comment.
I am interested to know how you found these version's are correct and what is someone have higher version ?
There was a problem hiding this comment.
I think his check uses the required version as the minimum version.
Actually, this is a good spot because the variables should be called MINIMUM_... instead of REQUIRED_....
| local node_version yarn_version docker_version | ||
| local errors=() | ||
|
|
||
| # Check Node.js version |
There was a problem hiding this comment.
Would love to see this in action attach a video in PR
web-server/src/assets/dora-icon.svg
Outdated
There was a problem hiding this comment.
What's this change ? Try not to commit it if not related.
|
@sahitya-chandra Please Add a video and some description to explain why this approach is good. |
Screencast.from.2024-10-17.19-38-30.webm@jayantbh @VipinDevelops I can't change my docker version but I can say it is working fine |
You removed this line, and then in the last commit added it back in a different way, why? :D 
|
|
I'd have literally approved and merged it without that change right now. :D |
|
@jayantbh Sir, I completly mised that |
fixed #168
@jayantbh Sir please check