On Sat, Jan 6, 2024 at 6:32 PM Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> On Sat, Jan 6, 2024 at 3:25 PM Tamino Bauknecht <dev@xxxxxx> wrote:
> > +create_fetch_all_expect () {
> > + cat >expect <<-\EOF || return 1
> > + ...
> > + EOF
> > +}
>
> This is really minor, but the `|| return 1` is superfluous. The `cat`
> command itself will exit with success or failure, and since it's the
> last command in the function, its return value will be the value
> returned by the function. Thus, there is no need to use `|| return 1`
> to signal failure when the `cat` command itself will do so anyhow.
Just for completeness, the other `|| return 1` in your patch...
> > + for r in one two three
> > + do
> > + git -C "$test_dir" remote add "$r" "../$r" || return 1
> > + done
... is necessary, thus correct, since shell for-loops don't terminate
automatically when a command in the loop body fails; the loop
continues regardless. Thus, this `|| return 1` ensures that we
correctly abort (signalling a failure) as soon as the error is
discovered.
