Hi Danh,

On Sun, 5 Apr 2020, Danh Doan wrote:

> On 2020-04-04 22:08:56+0200, Johannes Schindelin <Johannes.Schindelin@gmx.de> 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