Junio C Hamano <gits...@pobox.com> writes:

>> +            if (has_extension(de->d_name, ".idx") ||
>> +                has_extension(de->d_name, ".pack") ||
>> +                has_extension(de->d_name, ".keep"))
>> +                    string_list_append(&garbage, path);
>
> It might be OK to put .pack and .keep in the same "if (A || B)" as
> it may happen to be that they do not need any special treatment
> right now, but I do not think this is a good idea in general.

Actually I take this part back.  I can see that the condition part
grow over time but I do not think the body should.  That is the
whole point of collecting paths that cannot be judged as garbage by
themselves into a list; we shouldn't be doing anything else by
definition in the body.

Everything else I said in the review still stands, though..

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to