On Mon, Nov 22 2021, Glen Choo wrote:
> submoduleAlternateErrorStrategyDie::
> Advice shown when a submodule.alternateErrorStrategy option
> configured to "die" causes a fatal error.
> + submodulesNotUpdated::
> + Advice shown when a user runs a submodule command that fails
> + because `git submodule update` was not run.
> addIgnoredFile::
> Advice shown if a user attempts to add an ignored file to
> the index.
Does it need to be submodule*s*NotUpdated? I.e. the existing error is
submodule.. (non-plural), and we surely error on this per-submodule? The
plural would make senes if the advice aggregates them to the end, let's
look at the implementation...
> [...]
> + for (i = 0; i < submodule_entry_list->entry_nr; i++) {
> + submodules[i] = *submodule_from_path(
> + r, &super_oid,
> + submodule_entry_list->name_entries[i].path);
> +
> + if (repo_submodule_init(
> + &subrepos[i], r,
> + submodule_entry_list->name_entries[i].path,
> + &super_oid)) {
> + die(_("submodule %s: unable to find submodule"),
> + submodules[i].name);
> + if (advice_enabled(ADVICE_SUBMODULES_NOT_UPDATED))
> + advise(_("You may try initializing the submodules using 'git checkout %s && git submodule update'"),
> + start_name);
> + }
Uh, a call to advise() after die()? :) That code isn't reachable.
It would be good to add test for what the output is in the next
iteration, which would be a forcing function for making sure this code
works.
One thing I find quite annoying about submodules currently is the
verbosity of the output when we do N operations. E.g. I've got a repo
with 15-20 small submodules, cloning it prints out the usual "git clone"
verbosity, which isn't so much when cloning one repo, but with 15-20 it
fills up your screen.
Operations like these should also behave more like "git fetch --all",
surely? I.e. let's try to run the operation on all, but if some failed
along the way let's have our exit code reflect that, and ideally print
out an error summary, not N number of error() calls.
That would also justify the plural "submodulesNotUpdated", if such an
advise() printed out a summary of the N that failed at the end.
