search

fly-apps / dockerfile-rails

Provides a Rails generator to produce Dockerfiles and related files.
MIT License
474 stars 38 forks source link

Do not install node if bun is present #71

Closed luizkowalski closed 9 months ago

luizkowalski commented 9 months ago

If one uses Bun, the generator doesn't consider Node unnecessary and will add Node (and generate .node-version file) into the Dockerfile. As per the issue linked in #70, this can cause issues (e.g. assets won't precompile).

Fixes #70

rubys commented 9 months ago

Looking at the test failure, the removal of packages from apt-get install is goodness, but the omission of bun install is problematic:

https://github.com/fly-apps/dockerfile-rails/actions/runs/7254654968/job/19766937517?pr=71#step:6:729

It is possible to run individual tests and to capture results. See: https://github.com/fly-apps/dockerfile-rails/blob/main/README.md#testing

rubys commented 9 months ago
image
luizkowalski commented 9 months ago

test_bun and test_minimal are passing on my machine. Can you run the workflow? I haven't test all of them

luizkowalski commented 9 months ago

@rubys, thanks a lot for the collaboration invitation! I updated the PR description, is it fine to merge it?

rubys commented 9 months ago

If the tests pass and you are happy, merge when ready. In general, you don't need to ask for permission, if you see something that needs fixed, fix it; and I and others will do likewise.

Thanks for the contribution and welcome aboard!

luizkowalski commented 9 months ago

makes sense! I will give a try with the project I'm working with and if works as intended, I will merge