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

Fix reset.sh docker-compose call #1209

Closed
wants to merge 1 commit into from
Closed

Fix reset.sh docker-compose call #1209

wants to merge 1 commit into from

Conversation

matti
Copy link
Contributor

@matti matti commented Dec 20, 2021

No description provided.

@aminvakil
Copy link
Collaborator

This has been changed in #1179 .

Copy link
Collaborator

@aminvakil aminvakil left a comment

Choose a reason for hiding this comment

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

This should detect docker-compose version, some users (I won't say few or many as I have no idea) have migrated to docker compose > 2.0.0 and definitely not all of them have symlinked /usr/local/bin/docker-compose to plugin to make it compatible with this change.

@chadwhitacre
Copy link
Member

chadwhitacre commented Dec 20, 2021

This should detect docker-compose version

Agreed. It seems like it's time to teach reset.sh about install/_lib.sh, or factor out the dc stuff and have both source that (if all of _lib.sh is more than we want in reset.sh for some reason).

# Some environments still use `docker-compose` even for Docker Compose v2.
dc_base="$(docker compose version &> /dev/null && echo 'docker compose' || echo 'docker-compose')"
dc="$dc_base --ansi never --env-file ${_ENV}"
dcr="$dc run --rm"

@aminvakil
Copy link
Collaborator

I don't think of any particular reason not use the whole install/_lib.sh, but maybe later we want to add something to it, which breaks the reset.sh, so it's better to just copy docker-compose detection from it IMO.

@matti
Copy link
Contributor Author

matti commented Dec 21, 2021

fyi I am not going to do this extra changes, I just fixed the most obvious bug.

@aminvakil
Copy link
Collaborator

This should detect docker-compose version, some users (I won't say few or many as I have no idea) have migrated to docker compose > 2.0.0 and definitely not all of them have symlinked /usr/local/bin/docker-compose to plugin to make it compatible with this change.

@matti What part of this you disagree with?

@matti
Copy link
Contributor Author

matti commented Dec 21, 2021

I don't diasagree with anything - I just wont do anything else for this PR, feel free to do as much as you want.

@matti

This comment has been minimized.

@aminvakil
Copy link
Collaborator

Fine, thanks, I'll take it from here in #1215 .

@aminvakil aminvakil closed this Dec 21, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jan 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants