Re: [PATCH v2 0/7] commit-reach: terminate merge-base walk when one side is exhausted

[Derrick Stolee <stolee@gmail.com>]

On 6/24/2026 8:14 AM, Kristofer Karlsson via GitGitGadget wrote:
> commit-reach: terminate merge-base walk when one side is exhausted
> 
> Optimize paint_down_to_common() for merge-base queries that hit large
> one-sided histories.
I completed my review of this version. All of my comments are around
either making the commit history a little cleaner or expanding the
tests that use the trace2 data.

I believe that this code is _correct_ and could be shipped as-is. My
comments are focused on making it the best that it could be, with an
eye towards a cleaner final result or a more robust test setup.

The most actionable things are:

1. You can add tracing before the new tests, allowing the new tests
   to also check the step counts in their first versions and then
   get updated in the final patch to demonstrate how they change
   with that behavior change.

2. The t6600 helper 'test_all_modes' could set GIT_TRACE2_EVENT for
   each mode into a different trace file that can be scanned later.
   This will simplify your current tracing tests but also unlock
   easier tracing like this in the future.

3. The termination condition depending on min_generation could be
   refactored into paint_queue_get() to help make things even more
   obvious as to when we terminate. This should help with your
   concerns that you mentioned in response to patch 2/6 of the
   previous version:

> I am not 100% happy with the halt-condition placement yet --
> the existing loop in master already has several exit paths
> (while condition, min_generation break, FIND_ALL break) and I
> think there is an opportunity to consolidate them. But that is
> a separate discussion and I do not want to derail this series.
> I can propose some alternatives in a follow-up after this
> lands.

I then have some super minor comments around making the diffs
even easier to read, but they could be ignored as they are very
nit-picky. It's the kind of detail that I would try to resolve
if I was the author, but I'm _not_. You are. Your time is
valuable so make your own conclusions as to whether you want to
go down that road. You've already entertained my ideas around
updating the docs as the implementation changes.

Thanks,
-Stolee