Re: [PATCH v5] merge-tree: add -X strategy option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Phillip Wood <phillip.wood123@xxxxxxxxx> writes:

> ...
> This is the right place to call parse_merge_opt() but I think we
> should first check that the user has requested a real merge rather
> than a trivial merge.
>
> 	if (xopts.nr && o.mode == MODE_TRIVIAL)
> 		die(_("--trivial-merge is incompatible with all other options"));
>
> Otherwise if the user passes in invalid strategy option to a trivial
> merge they'll get an error about an invalid strategy option rather
> than being told --strategy-option is not supported with
> --trivial-merge.

I presume that another issue with not having that compatibility
checking with "--trivial" would be when the user passes a valid
strategy option and it would be silently ignored.

> Ideally there would be a preparatory patch that moves the switch
> statement that is below the "if(o.use_stdin)" block up to this point
> so we'd always have set o.mode before checking if it is a trivial
> merge. (I think you'd to change the code slightly when it is moved to
> add a check for o.use_stdin)

That sounds very good.

Thanks for a good "stepping back a bit" review.  Highly appreciated.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux