On 3/9/2022 8:16 AM, Ævar Arnfjörð Bjarmason wrote:
> Add a release_revisions() to various users of "struct rev_list" which
> requires a minor refactoring to a "goto cleanup" pattern to use that
> function.
A straight-forward refactor. I have a couple nits on this one.
> static int do_store_stash(const struct object_id *w_commit, const char *stash_msg,
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 09db2620829..19393da4e31 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -1222,6 +1222,7 @@ static int compute_summary_module_list(struct object_id *head_oid,
> struct strvec diff_args = STRVEC_INIT;
> struct rev_info rev;
> struct module_cb_list list = MODULE_CB_LIST_INIT;
> + int ret = 0;
Since we initialize ret = 0 here...
> @@ -1259,9 +1262,11 @@ static int compute_summary_module_list(struct object_id *head_oid,
> else
> run_diff_files(&rev, 0);
> prepare_submodule_summary(info, &list);
> + ret = 0;
... this is extraneous.
> die(_("unable to write %s"), get_index_file());
> -
> + ret = (result.clean == 0);
> +cleanup:
Here (and other places) I feel the loss of this empty line
above the goto label. It just seems helpful to help see
these labels a little more clearly if they have an empty
line first, instead of immediately following a statement.
Thanks,
-Stolee
