Hey Junio,
On Fri, Aug 26, 2016 at 9:59 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Pranit Bauva <pranit.bauva@xxxxxxxxx> writes:
>
>>> Also this version fails to catch "bisect reset a b c" as an error, I
>>> suspect.
>>
>> It didn't when I tried it right now. Could you please elaborate on why
>> you think it can fail? There might be a thing which I haven't tested.
>
> My bad. I just compared your bisect_reset() implementation that had
>
> if (no specific commit) {
> reset to the branch
> } else {
> reset to the commit
> }
>
> with the original
>
> case $# in
> 0) reset to the branch ;;
> 1) reset to the commit ;;
> *) give usage and die ;;
> esac
>
> and took the difference and reacted "ah, excess parameters are not
> diagnosed in this function".
>
> Your caller does complain about excess parameters without giving
> usage, and that is what I missed.
>
> I am not sure if you intended to change the behaviour in this case
> to avoid giving the usage string; I tend to think it is a good
> change, but I didn't see it mentioned in the proposed commit log,
> which also contributed to my not noticing the test in the caller.
I could include this in the commit message. Its not really something
which we would want to test in the function because to the function,
we are not passing the raw arguments. Since we are removing that check
from the function but including it in cmd_bisect__helper(), I will
talk about it in the commit message.
Regards,
Pranit Bauva
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html