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

Added compose v2 syntax compatibility #299

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

danielchodusov
Copy link

This PR provides compatibility with integrated "docker-compose" tool since Compose V2 as explained here https://docs.docker.com/compose/releases/migrate

Running the makefile now checks whether legacy docker installation is available or compose v2 compatible Docker is installed.

Copy link
Member

@kaplan-michael kaplan-michael left a comment

Choose a reason for hiding this comment

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

Check comment in line, othervise LGTM

@@ -23,15 +23,22 @@ endif
# Start the containers
.PHONY: compose-up
compose-up:
ifeq ($(DOCKER_COMPOSE),docker compose)
Copy link
Member

@kaplan-michael kaplan-michael Nov 21, 2024

Choose a reason for hiding this comment

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

you already echo docker compose into the $(DOCKER_COMPOSE) var, so I think it can be used directly here(as originally) and you don't need to check the contents to then replicate it in the condition block?
AFAIK the syntax after is the same, so it should be enough.
Not sure if it wouldn't cause issues, with it not having the full path to the docker binary?

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.

2 participants