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

[Raman Gupta <rocketraman@gmail.com>]

On 26/07/17 04:41 PM, Junio C Hamano wrote:
> Raman Gupta <rocketraman@gmail.com> writes:
> 
>> 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." ;;

I considered that but decided the non-collapsed version better
supports this list growing in the future.

> but still the repetition does look ugly.

Agreed.

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

Thanks.

Regards,
Raman