Re: [PATCH v8 00/10] commit-reach: terminate merge-base walk when one side is exhausted

From: Kristofer Karlsson <krka@spotify.com>
Date: Aug 12, 2026, 07:04 UTC
Message-ID: <CAL71e4NMdOeL0Mwngpw9qn=sSctFEVMPiYEiWQ=LT+Ct-mP02Q@mail.gmail.com>
In-Reply-To: <CABPp-BENLi7kBBu9QsN87aQY5C0kamzsKpXZTKYcHZk+WX11ng@mail.gmail.com>

On Wed, 12 Aug 2026 at 08:23, Elijah Newren <newren@gmail.com> wrote:

Show 9 quoted lines

>
> I am quite pleased with how this series has turned out.  Not only does
> it provide nice speedups, I think the structure of the series is
> particularly nicely set up in a way that helps guide the discovery of
> the idea behind the optimization for others to read, documents and
> tests everything logically and thoroughly, and was a pleasant read.
>
> I brought up several issues in previous versions, and this round
> addresses them all.

That's very much appreciated, I know this has been a lot of content to review and there has been some unfortunate churn, especially around the textual sections.

Show 8 quoted lines

> >       +For v1, `paint_down_to_common()` falls back to pure commit-date
> >      -+ordering via `compare_commits_by_commit_date`.  Because commit
> >      ++ordering via `compare_commits_by_commit_date`. Because commit
> >       +dates are not monotonic (clock skew, rebases, etc.), the queue
> >       +may visit commits out of topological order.
>
> Don't think for a second that I didn't notice you murdering these
> double spaces.  You villain!  ;-)

Yes, well spotted -- this was unfortunate but needed I think. I realized that this document was inconsistent, so I decided to standardize on single spaces because that was the smaller change and it also aligned with other technical documents (such as commit-graph.adoc) ...

> >   3:  f857577e0c !  3:  6208bcf3b3 t6600: add test cases for side-exhaustion edge cases
>
> ...and now you're just toying with us.  :-)

... and here I just restored your tests to their original glory, despite making the patch series as a whole feel a bit inconsistent.

Overall though, I wonder if it would be useful to codify a preference for double or single spacing after periods, even if that (obviously) does not imply we should reformat the entire code-base directly.

> Okay, my last two range-diff comments were just jokes, but more
> serious now: I think you did a nice job on the re-splitting.  Thanks
> for doing that!

Thank you, writing documentation feels harder (for me) than coding, but I'm somewhat happy with the end state now :)

- Kristofer