Re: [PATCH 2/2] pack-objects: only perform verbatim reuse on the preferred pack

From: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>
Date: Nov 22, 2024, 09:16 UTC
Message-ID: <9aad5aad-707e-4dcf-aa8d-8b3c82724133@app.fastmail.com>
In-Reply-To: <ZzX9vT4GVqCyfFUE@nand.local>

On Thu, Nov 14, 2024, at 14:40, Taylor Blau wrote:

Show 15 quoted lines

> On Wed, Nov 13, 2024 at 07:25:04PM -0500, Jeff King wrote:
>> I do think the explanation in the message of the first commit would be a
>> lot simpler if it were simply combined into this patch. With them split
>> you effectively have to explain the problem twice. I don't feel that
>> strongly about changing it, though.
>
> I always seem to go back and forth on that. I feel somewhat strongly
> that for complicated regression fixes that we should demonstrate the
> existing failure mode in a separate commit with a test_expect_failure.
> That forces the author to ensure they really understand the bug and can
> produce a minimal (or close to it) reproduction.
>
> It also makes it easier to demonstrate that the fix actually does what
> it says, instead of assuming that the test fails without the fix applied
> (and passes with it applied).

I recently made a parallel branch for a topic where the parallel branch had `test_expect_failure` for each commit (i.e. the commits had only `t/` changes). That ended up catching a bug I introduced when I tried to simplify the test: the test was OK on my topic branch but didn’t fail (`test_expect_failure`) on the parallel branch.

I use a worktree for `master` so at least I didn’t have to build specifically for that branch.

-- 
Kristoffer Haugsbakk