Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes: >> 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. I do not think running tests on stuff in contrib/ sends any such message. It primarily helps _us_ to catch more regressions than we may otherwise miss. By the way, this is not limited to contrib/; if we had tests for gitk, we would have caught the recent regression in "diff -m" before it got inflicted on the general public, but that would not have been just to help "gitk", but to help keep "diff -m" sane and stable [*]. By running tests on in-tree contrib/ like scalar, at least we would notice when we are making breaking changes. At least, the need for scalar (either for the API broken by such a change to be kept unchanged or done in a different way, or the code that uses the API on the scalar side to be updated) would be noticed earlier than stuff totally outside and not even in contrib/. Of course, you have to bear the burden of (A) changing the way scalar uses the API, or (B) participating in the design of the change to the API that may break scalar's use so that everybody including scalar would be happy, or both. It's not like I am responsible for everything that happens in the tree, and it is our shared responsibility to maintain the health of the codebase. It is not limited to stuff inside or outside contrib/. There are projects that want to use libgit.a by binding us as a submodule and without interacting with us very much. And they are on their own when we change the internals. Do you mean that you want to make scalar into the same status as they are? > 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. So that argues even more to have a way to make sure we catch unintended breakages by any future mindless tree-wide "clean-ups" and interface changes, no? [Footnote] * I just double checked the candidates for "test-extra" to see if they are meant to run with a random Git they happen to see on the $PATH, or they are designed to test with the version of Git we just built, and it seems it is the latter for the ones nominated in the test-extra patch. Otherwise it would indeed reduce the benefit in half---we are not helping to catch regressions in the core stuff in such a case.
