Re: [PATCH 08/24] revisions API users: add "goto cleanup" for release_revisions()
From: Derrick Stolee <hidden>
Date: 2022-03-09 19:09:01
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.
quoted hunk ↗ jump to hunk
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...
quoted hunk ↗ jump to hunk
@@ -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