Re: [PATCH v6 0/8] ci: add GitLab CI definition

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 9, 2023, 10:06 UTC
Message-ID: <xmqqjzqr1bs1.fsf@gitster.g>
In-Reply-To: <cover.1699514143.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> There's only a single change compared to v5 based on Chris' feedback,
> namely to move around a `return 1`. The newly extracted helper function
> `create_failed_test_artifacts()` indeed wasn't the correct place to put
> this error code.

The caller of the helper always signals failure by returning with 1 after calling this helper, and if we later add new callers, it is very plausible that the new callers also are calling the helper because they have noticed that the tests failed.

If the helper were not named "create_failed_test_artifacts" but were given a more appropriate name, say, "fail_with_test_artifacts", we wouldn't have said that the helper is not the right place to return with a failure status. So, quite honestly, the difference since the previous round is strictly a Meh to me.

But let's queue this version, because it is not making it worse ;-)

Thanks.