Re: [PATCH v7 07/10] commit-reach: introduce struct paint_state with per-side counters

On Fri, Aug 7, 2026 at 4:48 AM Kristofer Karlsson <[email protected]> wrote:
>
> On Fri, 7 Aug 2026 at 05:02, Elijah Newren <[email protected]> wrote:
> >
> > became -> become
>
> Good catch, will fix.
>
> > So: pop, clear, check the counters, and _then_ decrement the counters.
> > This means the zero-counter-check still include the just-popped
> > commit.  If the decrement were before the check, we'd actually just
> > barely miss the merge-base most the time, so this order is important.
>
> Yes, I should perhaps add a code comment for this to ensure it
> does not get corrupted in the future. Something like this:
>
>     /* must check exit conditions before decrementing counters
>        for the dequeued commit -- the counters may otherwise be
>        be temporarily zero until the commit has been processed
>        and its parent nodes have been enqueued.
>      */
>
> Or is it overkill?

My comment was meant more as a "review out loud; show how I'm thinking
about the patch as I read it" kind of comment rather than as a note of
something worth changing in the patch.  The order _is_ important here,
so the idea of adding a comment to reinforce it seems quite reasonable
to consider (I thought about changing my comment to ask for one when
reviewing), but I'm on the fence about whether it's important enough
or overkill.  I'm okay with the patch either way.

Although, if you do add one, as per the CodingGuidelines:

/*
 * Multi-line comments include their delimiters
 * on separate lines from the text.
 */

:-)