-
Notifications
You must be signed in to change notification settings - Fork 273
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
ci(dockerhub): fix garden version checks #4868
Conversation
Use new `garden version` command instead of discontinued `garden --version`.
93041fe
to
6676333
Compare
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.
Nice catch! 🎉
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.
Good catch 🌟
I think there's another issue here: Why does garden exit with code 0 when passing an unrecognized option? @vvagaytsev |
Yes, I think that should be changed too. It looks like the |
Technically, it would be a breaking change. This is what we have now:
# Unrecognized command:
> garden bad
# usage output omitted
> echo $?
1
# Unrecognized option:
> garden --bad
# usage output omitted
> echo $?
0
# Unrecognized _command_ option:
> garden deploy --bad
# usage output omitted
> echo $?
1
# Unrecognized command:
> garden bad
# usage output omitted
> echo $?
1
# Unrecognized option:
> garden --bad
# usage output omitted
> echo $?
0
# Unrecognized _command_ option:
> garden deploy --bad
# usage output omitted
> echo $?
1 So, it looks like the edge case is when we pass options to I think it makes sense to fix it in one of the next patch releases. |
I agree this should be fixed in a patch release, IMO this is not a breaking change but a bug! |
Created #4911. |
Use new
garden version
command instead of discontinuedgarden --version
.What this PR does / why we need it:
Fixes the way of checking the Garden version in the
docker-bake-test.sh
script.Without this PR the command fails silently with OK exit status like this:
This PR replaces the old-fashioned discontinued
garden --version
with the newgarden version
command.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer: