Hi Derrick, On Wed, Nov 11, 2020 at 12:48 PM Derrick Stolee <stolee@xxxxxxxxx> wrote: > > On 11/11/2020 1:35 PM, Elijah Newren wrote: > > On Wed, Nov 11, 2020 at 9:09 AM Derrick Stolee <stolee@xxxxxxxxx> wrote: > >> For the series as a whole I'd love to see at least one test that > >> demonstrates that this code does something, if even only for a very > >> narrow case. > >> > >> There's a lot of code being moved here, and it would be nice to have > >> even a very simple test case that can check that we didn't leave any > >> important die("not implemented") calls lying around or worse accessing > >> an uninitialized pointer or something. > > > > We absolutely left several die("not implemented") calls lying around. > > The series was long enough at 20 patches; reviewers lose steam at 10 > > (at least both you and Jonathan have), so maybe I should have left > > even more in there as an attempt to split up this series more. > > > > However, if you run the testsuite with GIT_TEST_MERGE_ALGORITHM=ort, > > then this series drops the number of failures in the testsuite from > > around 2200, down to 1500. So, there's about 700 testcases for you. > > Sorry that I'm jumping in to the series-of-series in the middle, so > I am unfamiliar with the previous progress and testing strategy. This Not a problem at all. Thanks much for jumping in and taking a look! You always provide some good feedback and suggestions. (Besides, those testcase changes have been spread over two and a half years...hard to stay on top of all of them.) > "number of test failures" metric is sufficient to demonstrate the > progress provided in this series. Perhaps it was even in your v1 cover > letter. Um, oops; it's not. I did mention there were still some "not implemented" messages left, but didn't mention the testcase counts. But even that mention is apparently in the v1 cover letter rather than v2, and v2 wasn't sent in-reply-to v1, so it's harder to catch that. Sorry about that; I'll include the testcase counts in the v3 cover letter.