On Tue, Nov 15, 2016 at 4:22 PM, David Turner <david.tur...@twosigma.com> wrote:
>>       msgs[ERROR_NOT_UPTODATE_DIR] =
>>               _("Updating the following directories would lose untracked
>> files in it:\n%s");
>> +     msgs[ERROR_NOT_UPTODATE_SUBMODULE] =
>> +             _("Updating the following submodules would lose modifications
>> in
>> +it:\n%s");
>
> s/it/them/

done, also fixed the existing ERROR_NOT_UPTODATE_DIR.

>> +             if (!S_ISGITLINK(ce->ce_mode)) {
>
> I generally prefer to avoid if (!x) { A } else { B } -- I would rather just 
> see if (x) { B } else { A }.

done.

>> +                             if (submodule_is_interesting(old->name, 
>> null_sha1)
>> +                                 && ok_to_remove_submodule(old->name))
>> +                                     return 0;
>> +                     }
>
> Do we need a return 1 in here somewhere?  Because otherwise, we fall through 
> and return 0 later.

Otherwise we would fall through and run

    if (errno == ENOENT)
        return 0;
    return o->gently ? -1 :
        add_rejected_path(o, error_type, ce->name);

which produces different results than 0?

Reply via email to