github action integration between g/gardenctl and g/homebrew-tap #14
Conversation
neo-liang-sap
force-pushed
the
master
branch
from
b1cd7f5 to
a77f3c6
Compare
| filteredTag=${1##*/} | ||
| tag=${filteredTag:-v0.17.0} | ||
| sha=${2:-8d33c751e8d32fe7fff15306c7de59cf15c45fb04e2f9abf988d3edd3f305cc4} | ||
| tag=${1:-v0.17.0} |
There was a problem hiding this comment.
why are you defaulting to 0.17.0?
There was a problem hiding this comment.
I have no preference on this ;) i set default 0.17.0 there because in your code the value is default to 0.17.0 so i kept it...shall i remove the default value?
There was a problem hiding this comment.
I think it makes sense to default to a more stable release if you want to keep the defaults. Otherwise, we can just error if the value is not provided.
| tag=${filteredTag:-v0.17.0} | ||
| sha=${2:-8d33c751e8d32fe7fff15306c7de59cf15c45fb04e2f9abf988d3edd3f305cc4} | ||
| tag=${1:-v0.17.0} | ||
| mac_sha=${2:-f6cbd049d200a1857e9bb89cd614f0f96851ef2db307a18609a18effbb4497eb} |
There was a problem hiding this comment.
what is this default sha for?
There was a problem hiding this comment.
again i have no preference on this default mac_sha , because i saw you have a default sha so i choose a default mac_sha, so just remove all the default value? thanks!
|
thanks @zanetworker , i removed all default values, add check whether they are empty , also fixed some typo |
What this PR does / why we need it:
github action integration between g/gardenctl and g/homebrew-tap
Which issue(s) this PR fixes:
Fixes gardener-attic/gardenctl#301
Special notes for your reviewer:
CC @dansible @ccwienk @zanetworker
this PR needs merge after gardener-attic/gardenctl#303
tested in my forked repo and seems OK
Release note: