Re: [PATCH v2] contrib/rerere-train: optionally overwrite existing resolutions

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 26, 2017, 20:41 UTC
Message-ID: <xmqq60efq6bo.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <32b61c4b-5e58-30d9-4345-8036bc805019@gmail.com>

Raman Gupta <rocketraman@gmail.com> writes:

Show 19 quoted lines

> On 26/07/17 02:05 PM, Junio C Hamano wrote:
>> I haven't tried this patch, but would this work well with options
>> meant for the 'git rev-list --parents "$@"' that grabs the list of
>> merge commits to learn from?  e.g.
>> 
>> 	$ contrib/rerere-train.sh -n 4 --merges master
>> 	$ contrib/rerere-train.sh --overwrite -n 4 --merges master
>> 	$ contrib/rerere-train.sh -n 4 --overwrite --merges master
>> 
>> I do not think it is necessary to make the last one work; as long as
>> the first two work as expected, we are good even if the last one
>> dies with a sensible message e.g. "options X, Y and Z must be given
>> before other options" (currently "X, Y and Z" consists only of
>> "--overwrite", but I think you get what I mean).
>
> You're right -- I didn't try all the cases. I wasn't able to figure
> out how to get `rev-parse --parseopt` to deal with this situation, so
> I did it manually. I'm not super-happy with the result, but it does
> work. Look for PATCH v3.

Yes, I think you could squash the two case arms in the later loop into one i.e.

	-h|--help|-o|--overwrite)
		die "please don't." ;;

but still the repetition does look ugly.

As a contrib/ material, I do not care too deeply about it, though.

Will queue.