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?