From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 13 Feb 2013 16:54:41 GMT
Subject: Re: [PATCH v4 3/4] count-objects: report garbage files in pack directory too
Message-ID: <7vpq049msu.fsf@alter.siamese.dyndns.org>
In-Reply-To: <7vehgkb43v.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@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.