Hi Ruslan,
On Wed, Oct 25, 2023 at 08:58:00AM +0000, Ruslan Yakauleu via GitGitGadget wrote:
> From: Ruslan Yakauleu <ruslan.yakauleu@xxxxxxxxx>
>
> A new option --ff-one-only to control the merging strategy.
> For one commit option works like -ff to avoid extra merge commit.
> In other cases the option works like --no-ff to create merge commit for
> complex features.
This seems like a pretty niche feature to want to introduce a new option
for. I would imagine the alternative is something like:
ff="--no-ff"
if test 1 -eq $(git rev-list @{u}..)
then
ff="--ff"
fi
[on upstream @{u}]
git merge "$ff" "$branch"
I don't have a great sense of how many users might want or benefit from
something like this. My sense is that there aren't many, but I could
very easily be wrong here.
In any case, my sense is that this is probably too niche to introduce a
new command-line option just to implement this behavior when the above
implementation is pretty straightforward. Regardless, here's my review
of the patch...
> @@ -631,6 +633,8 @@ static int git_merge_config(const char *k, const char *v,
> fast_forward = boolval ? FF_ALLOW : FF_NO;
> } else if (v && !strcmp(v, "only")) {
> fast_forward = FF_ONLY;
> + } else if (v && !strcmp(v, "one-only")) {
> + fast_forward = FF_ONE_ONLY;
The configuration handling and documentation all look good.
> } /* do not barf on values from future versions of git */
> return 0;
> } else if (!strcmp(k, "merge.defaulttoupstream")) {
> @@ -1527,6 +1531,18 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
> free(list);
> }
>
> + if (fast_forward == FF_ONE_ONLY) {
> + fast_forward = FF_NO;
> +
> + /* check that we have one and only one commit to merge */
> + if (squash || ((!remoteheads->next &&
> + !common->next &&
> + oideq(&common->item->object.oid, &head_commit->object.oid)) &&
> + oideq(&remoteheads->item->parents->item->object.oid, &head_commit->object.oid))) {
> + fast_forward = FF_ALLOW;
> + }
And this rather long conditional looks right, too. This patch could
definitely benefit from some tests, though...
Thanks,
Taylor
