-
Notifications
You must be signed in to change notification settings - Fork 0
Bump node to v22 #38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bump node to v22 #38
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the Docker image documentation to reflect the new supported Node.js version.
- Update Node.js version from v18 to v22 in the README.md file.
Files not reviewed (2)
- Dockerfile: Language not supported
- package.json: Language not supported
ain
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the bump! Some points made for considerations.
| apt-get install -y google-cloud-sdk nodejs yarn google-chrome-unstable --no-install-recommends && \ | ||
| apt-get autoremove && \ | ||
| rm -rf /var/lib/apt/lists/* | ||
| apt-get autoclean && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
package.json
Outdated
| "build": "docker build -t gcp-ruby/test -f ./Dockerfile .", | ||
| "run": "docker run --rm -it --entrypoint sh gcp-ruby/test:latest", | ||
| "clean": "docker image rm gcp-ruby/test:latest", | ||
| "full-test": "npm run test && npm run build && npm run run && npm run clean" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd avoid adding this over NPM, since it adds unnecessary complexity and failure layer as well as confusion (test vs full-test).
GitHub Actions is already testing against this and instead of local tests you could simply push the PR up and get it tested by Actions for you.
dockerlint was only added here because it's a Node module that does the linting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it, make sense
ain
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the work here Igor!
No description provided.