Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33509 )
Change subject: csb_patcher.sh: gets,checks,installs the coreboot and SeaBIOS patches ......................................................................
Patch Set 23:
(4 comments)
https://review.coreboot.org/c/coreboot/+/33509/7/csb_patcher.sh File csb_patcher.sh:
https://review.coreboot.org/c/coreboot/+/33509/7/csb_patcher.sh@10 PS7, Line 10:
Add a license?
Done.
https://review.coreboot.org/c/coreboot/+/33509/7/csb_patcher.sh@34 PS7, Line 34: printf "${bred}TERMINATED${bend}\n"
Here's what I'm using in my scripts. This avoids all the shellcheck warnings. […]
Thank you for your alternative variant. However I have some doubts about its' portability - [[ ]] is a bash'ism if I recall it correctly - and I'm trying to write this script as portable as possible, to work at max number of different shells. At the same time, I like the idea of a custom function encapsulating the ${NC} etc., I would borrow it if will need more colored and different prints.
https://review.coreboot.org/c/coreboot/+/33509/7/csb_patcher.sh@79 PS7, Line 79: else : return 0 : fi
Get rid of the else? This does the same thing. Same pattern below. […]
else is just in case I need to quickly insert a custom code to it, also like that both return lines are at the same offset.
https://review.coreboot.org/c/coreboot/+/33509/7/csb_patcher.sh@172 PS7, Line 172: ! -z
-n?
From one hand, -n is indeed better (it's a good idea to avoid ! if possible), but from the other hand: it's easy to memorize that -z means zero, while -n is a bit ambiguous: it means "not null" but easy to confuse with "null" if you don't spend much time in bash for a while.