Re: [RFC/PATCH] Makefile: add test-all target

Hi Junio,

On Wed, 8 Dec 2021, Junio C Hamano wrote:

We ship contrib/ stuff within our primary source tree but except for
the completion scripts that are tested from our primary test suite,
their test suites are not run in the CI.

Teach the main Makefile a "test-extra" target, which goes into each
package in contrib/ whose Makefile has its own "test" target and
runs "make test" there.  Add a "test-all" target to make it easy to
drive both the primary tests and these contrib tests from CI and use
it.

That sends a strong message that the stuff in contrib/ is now fully under
your maintenance, i.e. first-class supported.

If I were you, I wouldn't.

Junio C Hamano [off-list ref] writes:

quoted

That is an interesting way to demonstrate how orthogonal the issues
are, which in turn means that it is not such a big deal to add back
the coverage to the part that goes to contrib/scalar/.

I'd rather focus, _some_ focus, on the actual Scalar idea and code.

quoted

As the actual implementation, it is a bit too icky, though.

So, how about doing it this way?  This is based on 'master' and does
not cover contrib/scalar, but if we want to go this route, it should
be trivial to do it on top of a merge of ab/ci-updates and js/scalar
into 'master'.  Good idea?  Terrible idea?  Not good enough?

Peff mentioned a couple of times how tedious it is to address CI failures
e.g. in the Windows part of Git's CI runs.

So it makes only sense to avoid the same problem with contrib/scalar/
altogether, especially as long as you keep saying that you are still
uncertain whether it will make it into Git as a top-level command.

Which is a strong argument in favor of just leaving the CI part of
contrib/scalar/ out for now, and let it remain _my_ responsibility to
react to any build/test problems arising from unrelated patch series
entering `seen`.

Doing it that way would also have the benefit of allowing more focus on
the actual code in contrib/scalar/scalar.c.

Not that it needs more review, I don't think, as both Stolee and Elijah
gave their thumbs-up already, and I've not received any feedback that
would require further changes to `scalar.c`, at least as of _this_ patch
series.

Ciao,
Dscho