Junio C Hamano <gitster@xxxxxxxxx> writes: > Karthik Nayak <karthik.188@xxxxxxxxx> writes: > >> + - | >> + R=${CI_MERGE_REQUEST_TARGET_BRANCH_SHA-${CI_MERGE_REQUEST_DIFF_BASE_SHA?}} >> + >> + if test -z "$R" >> + then >> + echo "CI_MERGE_REQUEST_DIFF_BASE_SHA should always exist!" >> + exit 1 >> + fi > > The same comment applies as the previous step. > > R=${A-${B:?}} || exit > > should be sufficient. > > A demonstration. > > $ unset A; unset B; C=${A-${B:?}} && echo "C=$C" > bash: B: parameter null or not set > $ A=a; unset B; C=${A-${B:?}} && echo "C=$C" > C=a > $ unset A; B=; C=${A-${B:?}} && echo "C=$C" > bash: B: parameter null or not set > $ unset A; B=b; C=${A-${B:?}} && echo "C=$C" > C=b > $ A=a; B=b; C=${A-${B:?}} && echo "C=$C" > C=a > > Note that the broken case we do not see C=<value> becaues the > assignment fails with non-zero status. > > Thanks. Thanks Junio for explaining with examples, really nice of you! I'm on the fence with this, even the existing change from the previous more verbose code. I know this is shorter, but it is always more readable to use the longer version with 'test'. I find it hard to remember the specifics. But I don't really care which one makes it in the end. Let me know if you think it is worth a reroll. Thanks
Attachment:
signature.asc
Description: PGP signature