Re: [PATCH 1/5] grep: perform some minor code and comment cleanups

> On Mon, Mar 18, 2024 at 1:04 PM Dragan Simic <dsimic@manjaro.org> 
> wrote:
>> Move some variable definitions around, and reflow one comment block, 
>> to
>> make the code a bit neater after spotting those slightly unpolished 
>> areas.
>> There are no functional changes to the source code.
>> 
>> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
>> ---
>> diff --git a/builtin/grep.c b/builtin/grep.c
>> @@ -623,13 +623,13 @@ static int grep_tree(struct grep_opt *opt, const 
>> struct pathspec *pathspec,
>> -       struct repository *repo = opt->repo;
>> -       int hit = 0;
>> +       int hit = 0, name_base_len = 0;
>> +       int old_baselen = base->len;
>>         enum interesting match = entry_not_interesting;
>> +       struct repository *repo = opt->repo;
>>         struct name_entry entry;
>> -       int old_baselen = base->len;
>>         struct strbuf name = STRBUF_INIT;
>> -       int name_base_len = 0;
>> @@ -890,19 +890,15 @@ static int pattern_callback(const struct option 
>> *opt, const char *arg,
>> -       int hit = 0;
>> +       int hit = 0, seen_dashdash = 0, use_index = 1;
>>         int cached = 0, untracked = 0, opt_exclude = -1;
>> -       int seen_dashdash = 0;
>>         int external_grep_allowed__ignored;
>> +       int i, dummy, allow_revs;
>>         const char *show_in_pager = NULL, *default_pager = "dummy";
>>         struct grep_opt opt;
>>         struct object_array list = OBJECT_ARRAY_INIT;
>>         struct pathspec pathspec;
>>         struct string_list path_list = STRING_LIST_INIT_DUP;
>> -       int i;
>> -       int dummy;
>> -       int use_index = 1;
>> -       int allow_revs;
> 
> It's entirely subjective, of course, so no right-or-wrong answer, but
> I personally do not find that this change improves code quality or
> readability.
> 
> With my reviewer hat on, I spent an inordinate amount of time staring
> at this change trying to locate each variable's new location to verify
> that no initializers were dropped and that the declared type hadn't
> changed. Taking into consideration that reviewers are a limited
> resource on this project, I'd probably have skipped this patch
> altogether if I were doing this series unless these changes concretely
> help a subsequent patch.