From: Derrick Stolee Date: Wed, 24 Jun 2026 14:09:54 GMT Subject: Re: [PATCH v2 0/7] commit-reach: terminate merge-base walk when one side is exhausted Message-ID: In-Reply-To: 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