On Sun, Jul 04 2021, Alex Henrie wrote:
> I'm sending this patch as an RFC because I'm sure someone will find at
> least one thing that needs to be changed before it's committed.
FWIW that's true of most non-RFC patches :)
> +int error_ff_impossible(void)
> +{
> + error(_("Not possible to fast-forward, aborting."));
> + return -1;
> +}
Here's one, the idiom is just "return error", i.e it itself returns -1.
FWIW I'd consider doing:
/* earlier, static scope */
static const char error_ff_impossible = N_("Not possible...");
/* later, function scope */
return error(error_ff_impossible);
It's a small difference, but for translators who use the jump-to-source
while translating not having the indirection helps, and in this case
it's just used 3 times...
> [...]
> if (parent && parse_commit(parent) < 0)
> /* TRANSLATORS: The first %s will be a "todo" command like
> @@ -2764,7 +2769,7 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
> else if (!strcmp(key, "options.record-origin"))
> opts->record_origin = git_config_bool_or_int(key, value, &error_flag);
> else if (!strcmp(key, "options.allow-ff"))
> - opts->allow_ff = git_config_bool_or_int(key, value, &error_flag);
> + opts->fast_forward = git_config_bool_or_int(key, value, &error_flag) ? FF_ALLOW : FF_NO;
Since we're on nits, we try to wrap at 80 characters.
> +test_expect_success "rebase: impossible fast-forward rebase" '
> + test_config rebase.autostash true &&
> + git reset --hard &&
> + echo dirty >>file1 &&
> + (git rebase --ff-only unrelated-onto-branch || true) &&
Never "||" git commands, better as test_might_fail. We don't want to
hide segfaults.
I saw Phillip Wood had some comments on the rest, I didn't test this,
just skimmed it, but generally LGTM from a glance, not getting down to
the nuts of more deeply inspecting the logic.
