-
-
Notifications
You must be signed in to change notification settings - Fork 363
chore(docker): align Node version with engines requirement #1812
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
base: master
Are you sure you want to change the base?
chore(docker): align Node version with engines requirement #1812
Conversation
|
📝 WalkthroughWalkthroughUpdated the Dockerfile: bumped Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
What reviewer looks at during PR reviewThe following are ideal points maintainers look for during review. Reviewing these points yourself beforehand can help streamline the review process and reduce time to merge.
|
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.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
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.
|
@Harsh16gupta, please first familiarize yourself with the AsyncAPI Generator repository guidelines by carefully reading the CONTRIBUTING.md file: https://github.com/asyncapi/generator/blob/master/CONTRIBUTING.md |
|
@Adi-204 Thanks for pointing this out, and apologies for missing the process earlier. I have also moved the screenshot from the PR description to the corresponding issue and add an updated screenshot showing the result after applying the change. |
Adi-204
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.
lgtm!
Adi-204
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.
@Harsh16gupta This issue is now resolved—thanks for working on it.
Since this PR is quite small, I would recommend also addressing #1813
in the same PR if you’re planning to work on it?
|
Also read Note in point 1 in this comment and change title of the PR remove fix with chore. |
|
@Adi-204 I will work on it as directed |
b326872 to
0ae734d
Compare
|
|
@Adi-204 The remaining failure observed during Running the same test workspace in isolation ( I’ve intentionally not updated snapshots here to keep the scope limited to Docker and |
Adi-204
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.
@Harsh16gupta We cannot merge the PR until docker compose up runs successfully. Please take your time to make the required changes and ensure everything passes.





What
Align Docker base image with the Node version required by the project and fix Docker test failures caused by
turbo prune --dockerexcluding required runtime files.Fixes #1815
Fixes #1813
Why
1. Node version mismatch
The project requires:
However, the Dockerfile previously used Node 18, causing
npm cito fail with anEBADENGINEerror during Docker builds.2. Docker + turbo prune issue
When running tests inside Docker,
turbo prune --dockerexcludes files that are not part of the dependency graph but are still required at runtime, including:jest.config.base.js(required for Jest tests)packages/templates(required bybuild-templates.jsduring generator build)This caused Docker-based test and build failures, while local (non-Docker) runs succeeded.
Result
enginesrequirements.turbo prune.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.