From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 25 Apr 2023 18:32:25 GMT
Subject: Re: [PATCH 2/3] pack-bitmap.c: extract `fill_in_bitmap()`
Message-ID: <xmqqmt2vzubq.fsf@gitster.g>
In-Reply-To: <7624d3b5ba0415588a924aad2b855088e3d2028b.1682380788.git.me@ttaylorr.com>

Taylor Blau <me@ttaylorr.com> writes:

> To prepare for the boundary-based bitmap walk to perform a fill-in
> traversal using the boundary of either side as the tips, extract routine
> used to perform fill-in traversal by `find_objects()` so that it can be
> used in both places.

What is done is not a literal "extract", though.  Worth mentioning here?

> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
>  pack-bitmap.c | 66 +++++++++++++++++++++++++++++----------------------
>  1 file changed, 37 insertions(+), 29 deletions(-)
>
> +	if (base == NULL)
> +		base = bitmap_new();

Style?

> +	incdata.bitmap_git = bitmap_git;
> +	incdata.base = base;
> +	incdata.seen = seen;
> +
> +	revs->include_check = should_include;
> +	revs->include_check_obj = should_include_obj;
> +	revs->include_check_data = &incdata;
> +
> +	if (prepare_revision_walk(revs))
> +		die("revision walk setup failed");
> +
> +	show_data.bitmap_git = bitmap_git;
> +	show_data.base = base;
> +
> +	traverse_commit_list_filtered(revs, show_commit, show_object,
> +				      &show_data, NULL);

The original uses the variant without "_filtered" suffix.  In a
later update when the second caller is added, the new caller may
need to extend it, but shouldn't that wait until that step?

Thanks.