Skip to content
This repository has been archived by the owner on Oct 5, 2023. It is now read-only.

Simplify Dockerfiles. #10

Closed
wants to merge 1 commit into from
Closed

Conversation

reitermarkus
Copy link

Everything should still be working as before, just a little bit DRYer.

@CM2Walki CM2Walki self-assigned this Mar 28, 2020
@CM2Walki CM2Walki added the enhancement New feature or request label Mar 28, 2020
@CM2Walki
Copy link
Owner

CM2Walki commented Mar 28, 2020

Hey @reitermarkus!
Your Dockerfiles fail to build. See this screencap for more:

image

Make sure to actually test your Dockerfiles before you submit them for a PR.

EDIT:
To be frank, I'm not a big fan of having a big entry script. Anything that can be run before and be part of the image, should remain in the Dockerfile.

I'm quiet intrigued by the PR, considering it gets rid of some of the duplication (which could be eliminated by having a proper base image for all 3 images). However, I chose to have these duplicates to prevent the creation of a additional image layer.

@reitermarkus
Copy link
Author

reitermarkus commented Mar 28, 2020

Make sure to actually test your Dockerfiles before you submit them for a PR.

I did. The following commands run successfully:

docker build -t cm2network/csgo -f buster/Dockerfile .
docker build -t cm2network/csgo:sourcemod -f buster-sourcemod/Dockerfile .
docker build -t cm2network/csgo:metamod -f buster-sourcemod/Dockerfile .

to prevent the creation of a additional image layer

I think the number of layers is actually still the same in total.

@CM2Walki CM2Walki mentioned this pull request Jun 20, 2020
@CM2Walki
Copy link
Owner

Closed by #26. Dockerfiles have been split into something more maintainable.

@CM2Walki CM2Walki closed this Jun 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants