On Sat, Jan 6, 2024 at 3:25 PM Tamino Bauknecht <dev@xxxxxx> wrote:
> This commit introduces the new boolean configuration option fetch.all
> which allows to fetch all available remotes by default. The config
> option can be overridden by explicitly specifying a remote or by using
> --no-all.
> The behavior for --all is unchanged and calling git-fetch with --all and
> a remote will still result in an error.
>
> The config option was also added to the config documentation and new
> tests cover the expected behavior.
> Additionally, --no-all was added to the command-line documentation of
> git-fetch.
>
> Signed-off-by: Tamino Bauknecht <dev@xxxxxx>
> ---
> diff --git a/t/t5514-fetch-multiple.sh b/t/t5514-fetch-multiple.sh
> @@ -209,4 +218,156 @@ test_expect_success 'git fetch --multiple --jobs=0 picks a default' '
> +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.
For such a minor issue, I would typically say "not worth a reroll",
however, this sort of unnecessary code may confuse future readers into
thinking that something unusual and non-obvious is going on. As such,
it might be worth a reroll after all, but if you do choose to reroll,
wait until others have chimed in since they might have more to add
(either about this or other parts of the patch).
