mbox series

[0/7] Move the read_tree() function to its only user

Message ID 20210306193458.20633-1-avarab@gmail.com (mailing list archive)
Headers show
Series Move the read_tree() function to its only user | expand

Message

Ævar Arnfjörð Bjarmason March 6, 2021, 7:34 p.m. UTC 
This is a small cleanup series to remove move the last user of
read_tree() over to read_tree_recursive(), and while we're at it
adjust the API of read_tree_fn_t to its current use-case.

I found out after writing this that there had been a FIXME comment
(never made it into git.git) about this from mid-2009:
https://lore.kernel.org/git/1240014568-3675-1-git-send-email-pclouds@gmail.com/

Ævar Arnfjörð Bjarmason (7):
  tree.c API: move read_tree() into builtin/ls-files.c
  ls-files: don't needlessly pass around stage variable
  ls-files: remove cache juggling + sorting
  merge-ort: move cmp_cache_name_compare() from tree.c
  ls-files: refactor read_one_entry_quick() to use a strbuf
  tree.h API: remove support for starting at prefix != ""
  tree.h API: remove "stage" parameter from read_tree_recursive()

 archive.c          |  13 +++---
 builtin/checkout.c |   4 +-
 builtin/log.c      |   6 +--
 builtin/ls-files.c |  28 +++++++++++-
 builtin/ls-tree.c  |   4 +-
 merge-ort.c        |  10 ++++
 merge-recursive.c  |   4 +-
 tree.c             | 112 ++-------------------------------------------
 tree.h             |  11 +----
 9 files changed, 60 insertions(+), 132 deletions(-)

Comments

Elijah Newren March 6, 2021, 10:06 p.m. UTC | #1 
On Sat, Mar 6, 2021 at 11:35 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> This is a small cleanup series to remove move the last user of
> read_tree() over to read_tree_recursive(), and while we're at it
> adjust the API of read_tree_fn_t to its current use-case.
>
> I found out after writing this that there had been a FIXME comment
> (never made it into git.git) about this from mid-2009:
> https://lore.kernel.org/git/1240014568-3675-1-git-send-email-pclouds@gmail.com/
>
> Ævar Arnfjörð Bjarmason (7):
>   tree.c API: move read_tree() into builtin/ls-files.c
>   ls-files: don't needlessly pass around stage variable
>   ls-files: remove cache juggling + sorting
>   merge-ort: move cmp_cache_name_compare() from tree.c
>   ls-files: refactor read_one_entry_quick() to use a strbuf
>   tree.h API: remove support for starting at prefix != ""
>   tree.h API: remove "stage" parameter from read_tree_recursive()

I read over all 7 patches.  For the most part, they look good.

Some of the wording in the commit message for patch 6 seemed confusing
and could use some touch-ups; it may also be worth documenting when
the code stopped having any callers with a base other than "".

For patch 3 (the actual removal of read_tree()), either I'm
misunderstanding something, or the changes look problematic and
unsafe.  I'm hoping it's the former.