On Mon, Jan 25, 2016 at 9:47 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Christian Couder <christian.couder@xxxxxxxxx> writes:
>
>> diff --git a/read-cache.c b/read-cache.c
>> index 5be7cd1..a04ec8c 100644
>> --- a/read-cache.c
>> +++ b/read-cache.c
>> @@ -1497,10 +1497,23 @@ static struct cache_entry *create_from_disk(struct ondisk_cache_entry *ondisk,
>> return ce;
>> }
>>
>> -static void check_ce_order(struct index_state *istate)
>> +static void post_read_index_from(struct index_state *istate)
>> {
>> unsigned int i;
>>
>> + switch (git_config_get_untracked_cache()) {
>> + case -1: /* keep: do nothing */
>> + break;
>> + case 0: /* false */
>> + remove_untracked_cache(istate);
>> + break;
>> + case 1: /* true */
>> + add_untracked_cache(istate);
>> + break;
>> + default: /* unknown value: do nothing */
>> + break;
>> + }
>> +
>> for (i = 1; i < istate->cache_nr; i++) {
>> struct cache_entry *ce = istate->cache[i - 1];
>> struct cache_entry *next_ce = istate->cache[i];
>
> Bad manners.
>
> * The new code added to an existing function, unless there is a
> good reason, goes to the bottom. In this case, the verification
> of the ordering of cache entries and tweaking of UC extension are
> two unrelated things that can be independently done, and there is
> no justification why the new code has to come to top.
>
> * The old function name served as a good documentation of what it
> does. That is no longer the case. Each unrelated segment of
> this new function needs to be commented. Even better, perhaps
> leave the original check_ce_order() as-is, introduce a new
> function tweak_uc_extension(), and make the post_read_index()
> to be just two-liner function:
>
> static void post_read_index(struct index_state *istate)
> {
> check_ce_order(istate);
> tweak_uc_extension(istate);
> }
>
> That way the documentation value of each function that does one
> specific thing and named specific to its task will be kept, and
> there is no need for extra comments.
Ok, I will do that, thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html