From: René Scharfe <l.s.r@web.de>
Date: Fri, 26 Jun 2026 21:13:32 GMT
Subject: Re: [PATCH v3 5/8] commit-reach: introduce struct paint_state with per-side counters
Message-ID: <bd37b80d-9eff-496d-8f1f-436594968678@web.de>
In-Reply-To: <e82e0c72b6fc72b214f40efa9586c77790881f93.1782479286.git.gitgitgadget@gmail.com>

On 6/26/26 3:08 PM, Kristofer Karlsson via GitGitGadget wrote:
> 
> diff --git a/commit-reach.c b/commit-reach.c
> index f6a438550b..0f29b143bd 100644
> --- a/commit-reach.c
> +++ b/commit-reach.c
> @@ -97,6 +97,75 @@ static struct commit *nonstale_queue_get_dedup(struct nonstale_queue *queue)
>  	return commit;
>  }
>  
> +/*
> + * Priority queue with per-side commit counters for paint_down_to_common().
> + * Each non-stale queued commit occupies exactly one bucket: PARENT1-only,
> + * PARENT2-only, or both (a pending merge-base candidate).
> + */
> +struct paint_state {
> +	struct prio_queue queue;
> +	int p1_count;
> +	int p2_count;
> +	int pending_merge_bases;
> +};
Can they become negative?  Wouldn't size_t be a more natural fit,
matching nr from struct prio_queue?

And some bikeshedding:

Why abbreviate?  parent1_count and parent2_count would be slightly
easier to read and associate with PARENT1 and PARENT2.

And pending_merge_bases is a counter as well.  Why not call it
like that, pending_merge_base_count?   Well, that's pretty long.
both_count?  That's quite generic and nondescript.  Call the other
counters parents1 and parents2?  Nah.  Or parent1s and parent2s?
Not sure why this inconsistency bothers me to begin with.

René