Re: [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 12, 2020, 16:12 UTC
Message-ID: <xmqqy2kbqv7b.fsf@gitster.c.googlers.com>
In-Reply-To: <nycvar.QRO.7.76.6.2010111221350.50@tvgsbejvaqbjf.bet>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Also, the patch specifically adjusts the GitHub workflow itself.
> Therefore, unlike the `skip_good_tree()` function, it does not pretend to
> be generic (which `skip_good_tree()` really is not, as pointed out above).

I think skip_good_tree aspired to be a generic one, by having the "if we are not in travis nor GitHub actions, return early" at its very beginning. The person who adds support to GitHub workflow could have done one of two things.

One is to recognise the aspiration, and restructure existing skip_good_tree from

    skip_good_tree () {
	return if not travis and if not github actions

	bunch of code that happens to work only on travis
    }

to

    skip_good_tree () {
	if travis
	then
	    bunch of code that happens to work only on travis
	else if github actions
	    bunch of code that happens to work only with github
	fi
    }

without touching the caller. That lets skip_good_tree to be generic.

Another is to completely ignore that aspiration, maybe doing all that inside the workflow script (which by definition works only with github).

I think the latter (i.e. what the patch choose to do, which is not to bother with the ci/*.sh scripts at all) would be cleaner in this particular case, but then it would have made the intention more clear if the conditional at the beginning of skip_good_tree() were adjusted, perhaps?