On Thu, Aug 5, 2021 at 5:37 AM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote: > > The only tricky part of this series is the "clever" way of (ab)using > the object store and tags to detect copy/pasted tests. For this v3 > I've split that up into its own commit, see [56]/9 for the removal of > the copy/pasting and the assertion, respectively. I think this series would be better without that assertion (commit). While it is cool, it seems misplaced as it makes sense running ONLY when subtests had been added, and therefore probably as part of some "lint" job (ex: "as part of the static analysis CI job"), instead of every time this is called. FWIW, it is not only the conceptual misplacement that is a problem, but as coded a bug in git, could make these tests fail for a totally unrelated reason and therefore be confusing. Other than that and my comment about probably avoiding the in series rewriting of a function which might have been me not understanding the commit message, got my "Reviewed-by" for what it is worth. Carlo
