On Thu, Apr 18, 2024 at 12:53:02PM +0000, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
[snip]
> @@ -55,8 +58,9 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)
>  	else if (err)
>  		return 0;
>  
> -	for (i = 0; !result && i < values->nr; i++)
> -		result = run_command_on_repo(values->items[i].string, argc, argv);
> +	for (i = 0; (keep_going || !result) && i < values->nr; i++)
> +		if (run_command_on_repo(values->items[i].string, argc, argv))
> +			result = 1;

One thing that made me stop and think is whether the change in behaviour
here may negatively impact some usecases. Before this change we would
error out with the return code returned by the command that we have ran
in repositories. It makes total sense that we don't do that anymore with
`--keep-going`, because the result would likely be useless as all we
could do was to OR the result codes with each other.

But do we maybe want to make this conditional on whether or not the
`--keep-going` flag is set? So something like this:

```
for (i = 0; (keep_going || !result) && i < values->nr; i++) {
	int ret = run_command_on_repo(values->items[i].string, argc, argv);
	if (ret)
		result = keep_going ? 1 : ret;
}
```

Other than that this patch series looks good to me.

Patrick