Hi Danh, On Sun, 5 Apr 2020, Danh Doan wrote: > On 2020-04-04 22:08:56+0200, Johannes Schindelin <Johannes.Schindelin@xxxxxx> wrote: > > Hi Gábor, > > > > On Fri, 3 Apr 2020, SZEDER Gábor wrote: > > > > > > + CI_TYPE=github-actions > > > > + CI_BRANCH="$GITHUB_REF" > > > > + CI_COMMIT="$GITHUB_SHA" > > > > + CI_OS_NAME="$(echo "$RUNNER_OS" | tr A-Z a-z)" > > > > + test macos != "$CI_OS_NAME" || CI_OS_NAME=osx > > > > > > Hmm, if "macos" isn't not equal to $CI_OS_NAME, then set > > > CI_OS_NAME=osx. This is head-scratchingly backwards, and I think > > > > > > test "$CI_OS_NAME" = macos && CI_OS_NAME=osx > > > > > > would read better. > > > > I can understand where you come from, but your code is not `set -e` safe, > > which is the reason why I wrote the code this way (compare to the already > > existing code in the previous clause, which was copy-edited here). > > I certainly saw a shell that broke on > > set -e > test false && .. > > I couldn't recall it, though. > > Would it be OK if we change it this way: > > if "$CI_OS_NAME" = macos; then CI_OS_NAME=osx; fi We would have to fix quite a bit of code if we wanted to avoid the `<opposite> || <action>` pattern. If we _would_ need to (which I don't think we do), I'd prefer: case "$CI_OS_NAME" in macos) CI_OS_NAME=osx;; esac But again, we would have to clean up quite a few instances of this, I am sure, and it might not be worth the effort, given the short-and-sweet nature of the originally-proposed solution. Ciao, Dscho
