Re: [PATCH v2 1/2] for-each-repo: optionally keep going on an error

From: Patrick Steinhardt <ps@pks.im>
Date: Apr 19, 2024, 04:24 UTC
Message-ID: <ZiHyGFRPm_pwdGgC@tanuki>
In-Reply-To: <abd796894c857fc9ad96b9942089474df01f0506.1713444783.git.gitgitgadget@gmail.com>

On Thu, Apr 18, 2024 at 12:53:02PM +0000, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>

[snip]

Show 9 quoted lines

> @@ -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