On Sat, Jan 22 2022, Elijah Newren via GitGitGadget wrote:
> + /*
> + * TODO: Support subtree and other -X options?
> + if (use_strategies_nr == 1 &&
> + !strcmp(use_strategies[0]->name, "subtree"))
> + opt.subtree_shift = "";
> + for (x = 0; x < xopts_nr; x++)
> + if (parse_merge_opt(&opt, xopts[x]))
Better omitted WIP code in a non-RFC series?
> + die(_("Unknown strategy option: -X%s"), xopts[x]);
As a general issue with this series, die(), BUG() etc. messages should
start with a non-capital letter.
> + printf("%s\n", oid_to_hex(&result.tree->object.oid));
And for both this...
> + printf(_("Conflicts!\n"));
... and this we can just use puts(). For the former it's just less code,
but for the latter translators also don't need to see the always-there
\n in the translated message.
> +# This test is ort-specific
> +test "${GIT_TEST_MERGE_ALGORITHM:-ort}" = ort || {
Is this ${} trickery really needed? We're not testing with "set -u". So just:
if test "$GIT_..." != "ort"
then
...
fi
> +test_expect_success 'Barf on too many arguments' '
> + test_expect_code 129 git merge-tree --write-tree side1 side2 side3 2>expect &&
> +
> + grep "^usage: git merge-tree" expect
> +'
Nit: In most other tests these usage assertions are at the top of the
test, and for those we also make do with just testing the 129 exit code,
which is probably enough here too...
