On Sat, Mar 6, 2021 at 11:35 AM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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@xxxxxxxxx/ > > Æ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.