Re: [PATCH 2/2] merge-ort/merge-recursive: do report errors in `merge_submodule()`

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > In 24876ebf68b (commit-reach(repo_in_merge_bases_many): report missing
> > commits, 2024-02-28), I taught `merge_submodule()` to handle errors
> > reported by `repo_in_merge_bases_many()`.
> >
> > However, those errors were not passed through to the callers. That was
> > unintentional, and this commit remedies that.
> >
> > Note that `find_first_merges()` can now also return -1 (because it
> > passes through that return value from `repo_in_merge_bases()`), and this
> > commit also adds the forgotten handling for that scenario.
>
> Good clean-up.  But this "oops, we did not check for errors" makes
> me wonder if we are better off adopting "by default we assume an
> error, until we are sure we are good" pattern, i.e.
>
>         func()
>         {
>                 int ret = -1; /* assume worst */
>
>                 do stuff;
>                 if (...) {
>                         error(_("this is bad"));
>                         goto cleanup;
>                 }
>                 do stuff;
>                 if (...) {
>                         error(_("this is bad, too"));
>                         goto cleanup;
>                 }
>
>                 /* ok we are happy */
>                 ret = 0;
>
>         cleanup:
>                 release resources;
>                 return ret;
>         }
>
> The patch to both functions do make it appear that they are good
> candidates for application of the pattern to me.