Re: [PATCH 0/6] ls-tree.c: clean-up works

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jan 12, 2023, 15:02 UTC
Message-ID: <230112.868ri723p0.gmgdl@evledraar.gmail.com>
In-Reply-To: <20230112091135.20050-1-tenglong.tl@alibaba-inc.com>

On Thu, Jan 12 2023, Teng Long wrote:

Show 19 quoted lines

> From: Teng Long <dyroneteng@gmail.com>
>
> This patchset aims to do works on refactoring and cleanup
> of ls-tree.c.
>
> The patches can be viewed as two parts:
>
>       * Part 1: from Ævar Arnfjörð Bjarmason which mentioned in
>       his RFC patchset[1] and Ævar suggests to submit a separate clean-up
>       series. After I read them, I accept them all.
>
>       * Part 2: fÆvarrom me which introduced in my RFC patchset[2] and after
>       I accepted Ævar's patches, I keep two patches from [2], the left
>       patches in [2] may no longer be needed.
>
> [1] https://public-inbox.org/git/RFC-cover-0.4-00000000000-20221117T134528Z-avarab@gmail.com/
> [2] https://public-inbox.org/git/20221117113023.65865-1-tenglong.tl@alibaba-inc.com/
>
> Thanks.

Thanks for picking this up. It would be great to have this queued up.

It looks good to me, although that's mostly self-approving, as the
range-diff between this and my [1] (and your description, but I wanted
to sanity check it) shows it's mostly the same changes:
	
p	1:  a7f86973d57 ! 1:  5004f03c4d2 ls-tree: don't use "show_tree_data" for "fast" callbacks
	    @@ Commit message
	     
	         1. https://lore.kernel.org/git/cover-0.7-00000000000-20220310T134811Z-avarab@gmail.com/
	     
	    +    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
	    +    Signed-off-by: Teng Long <dyronteng@gmail.com>
	    +
	      ## builtin/ls-tree.c ##
	     @@ builtin/ls-tree.c: static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,
	      	return recurse;
	2:  b046e6f5692 ! 2:  de5b4d0767a ls-tree: use a "struct options"
	    @@ Commit message
	         before, now it's just being brought to the surface.
	     
	         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
	    +    Signed-off-by: Teng Long <dyroneteng@gmail.com>
	     
	      ## builtin/ls-tree.c ##
	     @@
	3:  329bc581b79 ! 3:  8f4b59183f6 ls-tree: fold "show_tree_data" into "cb" struct
	    @@ Commit message
	         we'll use only for that callback.
	     
	         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
	    +    Signed-off-by: Teng Long <dyroneteng@gmail.com>
	     
	      ## builtin/ls-tree.c ##
	     @@ builtin/ls-tree.c: struct ls_tree_options {
	4:  de4f89645fa ! 4:  d701aceb101 ls-tree: make "line_termination" less generic
	    @@ Commit message
	         API itself does much the same thing.
	     
	         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
	    +    Signed-off-by: Teng Long <dyroneteng@gmail.com>
	     
	      ## builtin/ls-tree.c ##
	     @@ builtin/ls-tree.c: static void expand_objectsize(struct strbuf *line, const struct object_id *oid,
	-:  ----------- > 5:  72bf99a1c74 ls-tree: cleanup the redundant SPACE
	-:  ----------- > 6:  978b89e5ea8 t3104: remove shift code in 'test_ls_tree_format'

I approve of that addition of the missing Signed-off-by (which I just forgot about).

I left comments on the 2x new patches, but they were both nit-y suggestions for maybe rephrasing the commit messages a bit, but as noted I don't think it's worth another re-roll (just maybe if others comment on the other patches and/or find other issues worth re-rolling).