Re: [PATCH 0/7] Move the read_tree() function to its only user
From: Elijah Newren <hidden>
Date: 2021-03-06 22:07:44
On Sat, Mar 6, 2021 at 11:35 AM Ævar Arnfjörð Bjarmason [off-list ref] 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/ (local) Æ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.