Subject: Re: [PATCH v1 1/1] list-objects.c: traverse_trees_and_blobs: rename and tree-wide

From: Junio C Hamano <[email-masked]>
Date: Wed, 11 Aug 2021 11:00:00 -0700

Teng Long <[email-masked]> writes:

> Subject: Re: [PATCH v1 1/1] list-objects.c: traverse_trees_and_blobs: rename and tree-wide

Sorry but I do not quite get the " and tree-wide" part of the title.

> Function `traverse_trees_and_blobs` not only works on trees and blobs,
> but also on tags, the function name is somewhat misleading. This commit
> rename it to `traverse_trees_and_blobs_and_tags`.

This looks pretty much "Meh" to me.

The current name might mislead people that we are limited to only
two types, but I doubt the risk of misleading is worth reducing with
such an overly long name.  The original is long enough X-<.

When we introduced it at 91904f56 (list-objects.c: factor out
traverse_trees_and_blobs, 2017-11-02), we may have been better to
call it traverse_non_commits().  The idea of traverse_commit_list(),
which is its primary caller, is for its main loop to iterate over
commits and process them one by one in each iteration, and process
objects of other types discovered therein by calling this function
once per each commit [*1*].

s/trees_and_blobs/non_commits/ will result in a name that is much
shorter and to the point, I think.

[Footnote]

*1* The call to it at the end is meant to sweep anything
leftover---we will not discover any tag while in the main loop of
the caller that iterates over commits (as they cannot contain any
tag in there).

> Signed-off-by: Teng Long <[email-masked]>
> ---
>  list-objects.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/list-objects.c b/list-objects.c
> index 7f404677d5..37a37fc502 100644
> --- a/list-objects.c
> +++ b/list-objects.c
> @@ -334,7 +334,7 @@ static void add_pending_tree(struct rev_info *revs, struct tree *tree)
>  	add_pending_object(revs, &tree->object, "");
>  }
>  
> -static void traverse_trees_and_blobs(struct traversal_context *ctx,
> +static void traverse_trees_and_blobs_and_tags(struct traversal_context *ctx,
>  				     struct strbuf *base)
>  {
>  	int i;
> @@ -407,9 +407,9 @@ static void do_traverse(struct traversal_context *ctx)
>  			 * needs a reallocation for each commit. Can we pass the
>  			 * tree directory without allocation churn?
>  			 */
> -			traverse_trees_and_blobs(ctx, &csp);
> +			traverse_trees_and_blobs_and_tags(ctx, &csp);
>  	}
> -	traverse_trees_and_blobs(ctx, &csp);
> +	traverse_trees_and_blobs_and_tags(ctx, &csp);
>  	strbuf_release(&csp);
>  }