search

realestate-com-au / shush

It's a secret.
169 stars 36 forks source link

README: Clean up Dockerfile example #34

Closed toothbrush closed 2 years ago

toothbrush commented 2 years ago

This way one doesn't need a build arg, and it's a self-contained copy-pasta blob.

tigris commented 2 years ago

G'day Paul.

I'm curious under what architectures is TARGETARCH not accurate for you? My fear is this change breaks if/when we start to release for other architectures... which to be fair is unlikely 🤣

toothbrush commented 2 years ago

Hey there!

I'm curious under what architectures is TARGETARCH not accurate for you? My fear is this change breaks if/when we start to release for other architectures... which to be fair is unlikely 🤣

I think this might be a situation where i've learned a thing about Docker! I did not know, but it appears as though under Docker Buildx certain build args are available by default. Unfortunately that document doesn't provide an exhaustive list, but this Dockerfile reference looked helpful to me. Dropping links here for others who might stumble across this.

I'll try it out tomorrow! I opened this PR because i thought i'd have to determine and then pass in the TARGETARCH variable when invoking docker build ... but this does sound preferable. Apologies for the noise, and thanks for teaching me a thing about new-gen Docker 🙌

tigris commented 2 years ago

All good. Yeah I guess we should mentioned a minimum version of docker or something in the document.

I think ENABLE_DOCKER_BUILDX=true has been the default in docker-desktop and docker-ce for a while now. It enables the ability to do docker --platform .... build when you do builds, or determines a default platform for you, and makes that available at buildtime via TARGETARCH. As well as a bunch of other things.

Prior to about 18 months ago you had to install buildx manually and force enable it, but it's pretty standard and default now.