-
-
Notifications
You must be signed in to change notification settings - Fork 952
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
Update NPM installation for containers #1849
Conversation
Build checks have not completed. Possible reasons for this are:
|
The docker container doesn't build for some reason, could you look into that? Also, I think that it would be better to only update the main docker image as we don't want to have to maintain the devcontainer separately, and this doesn't fix all the issues with it. |
911bfda
to
ba9129f
Compare
Hmm; might be some caching-related thing. It does build on my machine, although I'm using Podman rather than Docker. I'll see what I can get figured out. |
ba9129f
to
ce87693
Compare
Looks like the package caching was causing non-deterministic build failures of some kind. Not sure why, but it's not a necessary component. |
Yeah, the caching doesn't seem that important to me, because it only makes a difference when rebuilding the docker image, which isn't necessary very often. It was also out of scope for this PR. |
Agreed; it mostly just helps when actively tinkering with the |
If I understand correctly, we need to apply this change because the old way to install nodejs is now deprecated? Also, now, the container seems to build on the CI. Have you check that the Docker container works as expected? |
Yep; this is a fix for #1846.
I'm able to start building, but something's off with
That seems to be broken even if I build using the old container, though. |
Why there is no package.json and package-lock.json? I'm facing similar issues and updating one library seems to break another. |
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.
Looks good to me. The issue mentioned in the comments has been resolved already.
This resolves a deprecation warning with the
nodejs
package (#1846).