-
Notifications
You must be signed in to change notification settings - Fork 1
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
Handle docker compose and docker space compose #85
base: main
Are you sure you want to change the base?
Handle docker compose and docker space compose #85
Conversation
Thanks for the PR @vandaimer 😄 🚀 |
@@ -2,8 +2,9 @@ | |||
|
|||
ROOT="$(dirname $( cd -- "$(dirname "$0")" >/dev/null 2>&1 ; pwd -P ))" | |||
$ROOT/docker/env.sh | |||
. $ROOT/docker/base.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is needed to run with .
? (same as source
, right?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docker/base.sh
Outdated
|
||
DOCKER_COMPOSE_BIN="docker-compose" | ||
DOCKER_COMPOSE_FROM_DOCKER="docker compose" | ||
DOCKER_COMPOSE="" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of storing a variable, what you think about echoing the value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First, to be honest, not sure if is really needed this line 5, I could remove that.
Second, I can do this here base.sh
if [ -x "$(command -v $DOCKER_COMPOSE_BIN)" ]; then
echo $DOCKER_COMPOSE_BIN
else
echo $DOCKER_COMPOSE_FROM_DOCKER
fi
And on down.sh and run.sh
DOCKER_COMPOSE=$("$ROOT/docker/base.sh")
Works!
My concern with this change is that the file's name is base
, and the idea is to add/move more things into it, for example, $ROOT/docker/dockerfile
that has been repeated many times, but the value is the same for all usages.
So, to return (echoing) more than one value from base
would be difficult from the file reading the returns or I just don't know how to do it.
So, let me know what is your idea and let's make that happen :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I echo the value instead assigned into a variable, then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still did not have time to review this one yet 😬
Once I review I will answer you!
What is the goal of this PR? | Qual é o objetivo deste PR?
Solve this issue
Issue with new Docker compose command
Create a script that can decide which "docker compose" binary should be used.
The priority still on the
docker-compose
binary. Ifdocker-compose
doesn't exist, then, usedocker compose
base.sh
can be used for other things later, like define "default" values and load from the base.What has been done to achieve the goal? | O que foi feito para alcançar o objetivo?
Check if
docker-compose
binary exists. If yes, use it, if not, fallback todocker compose
How to test if the changes work? | Como testar se as alterações funcionam?
run.sh
and add another line like:echo $DOCKER_COMPOSE
after line 6docker/run.sh
docker-compose
, the output from the line above will be the path of yourdocker-compose
docker-compose
, the output from the line above will bedocker compose
docker-compose
changing the content ofDOCKER_COMPOSE_BIN
onbase.sh
line 3 and rundocker/run.sh
again