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

Remove version constraints on container images #549

Merged
merged 2 commits into from
Sep 29, 2023
Merged

Remove version constraints on container images #549

merged 2 commits into from
Sep 29, 2023

Conversation

f1rstlady
Copy link
Contributor

This fixes errors during the build of this image, occuring due to non-existent version tags.

Copy link

@Jarmos-san Jarmos-san left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make use build-time variables instead? That would allow for more flexibility to build the Docker Image. See #550 for some ideas I had in mind.

@f1rstlady
Copy link
Contributor Author

Can you make use build-time variables instead? That would allow for a more flexible to build the Docker Image. See #550 for some ideas I had in mind.

Do we need this flexibility? I'm asking because I prefer to add the minimal needed abstraction only. In addition, I suggest to distinguish between fixing and enhancing the image.

Other ideas for enhancements:

  1. Do we need the debian-based builds? Afaik, docker is used only by the pre-commit hook, which in turn uses the alpine-based image only.
  2. @Kampfkarren As the build pipeline shows, the built images are pushed to the Docker hub. But under which account? Shall we document the location of the builds?

@Jarmos-san
Copy link

Do we need this flexibility? I'm asking because I prefer to add the minimal needed abstraction only. In addition, I suggest to distinguish between fixing and enhancing the image.

Don't fix what's not broken is definitely the easier way around the issue with the Docker build issue right now tbh. The flexible enhancements I suggested on the other hand is to future-proof a potential scenario wherein someone had to test the build against a different Rust Docker Image for whatever reason.

For example, what if someone had to quickly check if a certain feature works on a particular Image or perhaps a specific version of Rust? Passing the build-time variables to docker would allow us to do that.

The other intention I had in mind was, since the current Dockerfile is worked on, why not do it the proper way right now instead of waiting for someone else to do it in the future? It'll allow Kamp to be relieved off of some maintenance burden as well.

@Kampfkarren
Copy link
Owner

@f1rstlady My assumption for the account is going to be boyned. This looks fine

@Kampfkarren Kampfkarren enabled auto-merge (squash) September 12, 2023 04:51
This fixes errors during the build of this image, occuring due to
non-existent version tags.
auto-merge was automatically disabled September 20, 2023 10:01

Head branch was pushed to by a user without write access

@Kampfkarren Kampfkarren enabled auto-merge (squash) September 29, 2023 04:14
@Kampfkarren Kampfkarren merged commit a6c7a1e into Kampfkarren:main Sep 29, 2023
11 of 12 checks passed
@f1rstlady f1rstlady deleted the fix/container-image-version branch September 29, 2023 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants