Skip to content
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

feat: add a few docker-compose env vars to control the build + docs #31450

Closed
wants to merge 1 commit into from

Conversation

mistercrunch
Copy link
Member

Living in docker/compose over the past few days, I figure it'll be good to attach build
args to env vars so it's easy to turn things on and off.

As part of this, I'm turning off installing chromium in the build as in most cases developers
won't be working on something that requires it. If you need though, it's
easy to set an env var to switch it on.

Should speed up the local builds a little and make images significantly
smaller locally.

Living in docker/compose over the past few days, I figure it'll be good to attach build
args to env vars so it's easy to turn things on and off.

As part of this, I'm turning off installing chromium in the build as in most cases developers
won't be working on something that requires it. If you need though, it's
easy to set an env var to switch it on.

Should speed up the local builds a little and make images significantly
smaller locally.
@github-actions github-actions bot added the doc Namespace | Anything related to documentation label Dec 14, 2024
@rusackas
Copy link
Member

I'm not sure if this qualifies as "breaking" for certain edge cases, but it could be added to the board if it seems like we ought to play it safe. Today is the deadline for such proposals.

@mistercrunch
Copy link
Member Author

mistercrunch commented Dec 17, 2024

Oh I think the bulk of the changes here got merged in my other PR here. I kept the default in Dockerfile and set new aligned defaults in docker-compose.yml .

The main change that may affect people is that Chromium is no longer in the dev docker image, but only for docker compose. Adding it back if/when needed is now well documented.

The benefit is we all get leaner dev environments that build/load faster.

I could add a note in UPDATING.md but that shouldn't affect docker images, only the dev/docker-compose context

@sadpandajoe
Copy link
Member

@mistercrunch looks like there are some failures when pushing docker in master looking at the master commits. Not sure why they aren't failing in branches but wonder if we could catch these things before they merge

@mistercrunch
Copy link
Member Author

mistercrunch commented Dec 17, 2024

Yeah I saw, will fix tomorrow, issue is around the fact that we --load now for docker-compose to run test, and that --load is not allowed for multi-platform builds, and it's set up to do multi-platform builds on master and single-platform on PRs (explains why I didn't catch it on my PR's CI). I'll just have to not --load and add a step to docker pull in CI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Namespace | Anything related to documentation size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants