On 5/19/2021 10:14 AM, Jeff King wrote: > On Wed, May 19, 2021 at 09:58:50AM -0400, Derrick Stolee wrote: > >>> (Note that this isn't a pure revert; the previous commit added a test >>> showing the regression, so we can now flip it to expect_success). >> >> Keeping the test is excellent, because it gives us a way to confirm >> that a second attempt at a fix is at least as good as the first. >> >> The only thing that could improve this situation is to add a test >> that checks the bug that the previous version introduced, so that >> the next round doesn't repeat the mistake. That can be deferred >> because it is more important that we get this fix in time for the >> next release candidate. > > Re-reading what I wrote, I think "the previous commit" may be ambiguous. > The original commit which introduced the bug (and which we're reverting > here) didn't include a test at all. In patch 1/2 of this series (what > I'm calling "the previous commit"), I provided a test which shows the > regression. And now this revert shows that we fixed it (by flipping from > expect_failure to expect_success). > > So I think I've already done what you're asking (if I understand it > correctly). Ah. For some reason my email client didn't thread your messages together, so I saw this as a one-off patch (ignoring the 2/2 part of the message, of course). Thanks, -Stolee
