Subject: Re: [PATCH v3 4/5] MyFirstObjectWalk: fix description for counting omitted objects

From: Junio C Hamano <[email-masked]>
Date: Mon, 25 Mar 2024 10:25:52 -0700

Dirk Gouders <[email-masked]> writes:

> Before the changes to count omitted objects, the function
> traverse_commit_list() was used and its call cannot be changed to pass
> a pointer to an oidset to record omitted objects.
>
> Fix the text to clarify that we now use another traversal function to
> be able to pass the pointer to the introduced oidset.
>
> Helped-by: Kyle Lippincott <[email-masked]>
> Signed-off-by: Dirk Gouders <[email-masked]>
> ---
>  Documentation/MyFirstObjectWalk.txt | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt
> index a06c712e46..811175837c 100644
> --- a/Documentation/MyFirstObjectWalk.txt
> +++ b/Documentation/MyFirstObjectWalk.txt
> @@ -754,10 +754,11 @@ points to the same tree object as its grandparent.)
>  === Counting Omitted Objects
>  
>  We also have the capability to enumerate all objects which were omitted by a
> -filter, like with `git log --filter=<spec> --filter-print-omitted`. Asking
> -`traverse_commit_list_filtered()` to populate the `omitted` list means that our
> -object walk does not perform any better than an unfiltered object walk; all
> -reachable objects are walked in order to populate the list.
> +filter, like with `git log --filter=<spec> --filter-print-omitted`. To do this,
> +change `traverse_commit_list()` to `traverse_commit_list_filtered()`, which is
> +able to populate an `omitted` list. Note that this means that our object walk

"this means that" could be rephrased in a way a bit more helpful and
to readers with clarity, perhaps:

	Note that our object walk will not perform any better than
	an unfiltered walk with this function, because all reachable
	objects need to be walked in order to ...

> +will not perform any better than an unfiltered object walk; all reachable
> +objects are walked in order to populate the list.

Other than that, looking very good.

Thanks, both.

>  First, add the `struct oidset` and related items we will use to iterate it:
>  
> @@ -778,8 +779,9 @@ static void walken_object_walk(
>  	...
>  ----
>  
> -Modify the call to `traverse_commit_list_filtered()` to include your `omitted`
> -object:
> +Replace the call to `traverse_commit_list()` with
> +`traverse_commit_list_filtered()` and pass a pointer to the `omitted` oidset
> +defined and initialized above:
>  
>  ----
>  	...