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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 25, 2024, 17:25 UTC
Message-ID: <xmqq34semden.fsf@gitster.g>
In-Reply-To: <cfa4b9ce503e98035d3ce09b0c9e00bcfb6ff70a.1711368499.git.dirk@gouders.net>

Dirk Gouders <dirk@gouders.net> writes:

Show 28 quoted lines

> 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 <spectral@google.com>
> Signed-off-by: Dirk Gouders <dirk@gouders.net>
> ---
>  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.

Show 14 quoted lines

>  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:
>  
>  ----
>  	...