On Thu, Jan 12 2023, Teng Long wrote: > From: Teng Long <dyroneteng@xxxxxxxxx> > > 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@xxxxxxxxx/ > [2] https://public-inbox.org/git/20221117113023.65865-1-tenglong.tl@xxxxxxxxxxxxxxx/ > > 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@xxxxxxxxx/ + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> + Signed-off-by: Teng Long <dyronteng@xxxxxxxxx> + ## 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@xxxxxxxxx> + Signed-off-by: Teng Long <dyroneteng@xxxxxxxxx> ## 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@xxxxxxxxx> + Signed-off-by: Teng Long <dyroneteng@xxxxxxxxx> ## 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@xxxxxxxxx> + Signed-off-by: Teng Long <dyroneteng@xxxxxxxxx> ## 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).