search

DieterReuter / image-builder-rpi64

Build SD card image for Raspberry Pi 3 64bit
MIT License
157 stars 37 forks source link

More Composability #77

Closed Magnitus- closed 6 years ago

Magnitus- commented 6 years ago
Magnitus- commented 6 years ago

@DieterReuter Re-thinking about this pull request, I'm not 100% happy with it.

I think the change in the nature of the environment variables is good as it allows more composition in the artifact names (for example, in the project as it currently stands, a debian arm 64 bits root fs is assumed, but with my modification, you can override that to any root fs which makes sense).

For having environment variables in a config file vs having them in the Dockerfile, it depends on your view of the project I think (either as the master orchestration of the HypriotOS build in which case a config file makes sense or simply as the final step in the HypriotOS build in which case variables in the Dockerfile make more sense).

With my current modifications, however, it's 75% in the Dockerfile and 25% in the config file which suits my needs (all the environment variables I need are in the Dockerfile), but for maintainers, it just puts the config in two places. I could move the remaining variables from the config file to the Dockerfile, but it might be unsatisfying for those who view this project as a top-level orchestrator.

Instead, what I could do as a compromise is put the defaults back in the config file, but only source the config file if it exists (if it doesn't, then all environment variables have to be passed to the container at run-time).

Let me know what you think.

DieterReuter commented 6 years ago

I would prefer to have all the variables at a single place. For example, when I want to upgrade the os-rootfs, I have to change the version number in Dockerfile and the file checksum in version.config.

Right now there are only 4-5 settings left in version.config, I think it's best to move them over to the Dockerfile. Then we'll have everything together and it's simple to change/update and makes maintaining way easier.

Magnitus- commented 6 years ago

Sounds good, I'll make the adjustment when I get back home.

DieterReuter commented 6 years ago

Thanks, this LGTM now!