Re: [PATCH 2/2] git.c: binary-search builtin commands

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

 



Nguyán ThÃi Ngác Duy  <pclouds@xxxxxxxxx> writes:

> An obvious implication of this patch: commands array must be in correct
> order.
>
> Signed-off-by: Nguyán ThÃi Ngác Duy <pclouds@xxxxxxxxx>
> ---
>   2011/2/15 Junio C Hamano <gitster@xxxxxxxxx>:
>   > I did this myself the other day, as I think it simply is a good project
>   > hygiene.  If this were 1/2 of a series followed by 2/2 that runs binary
>   > search in the table, that would make it make more sense ;-)
>   
>   I did think the array was binary-searched and nearly claimed "git-stage
>   won't work because it's in wrong order".

Heh, that "binary search" was a tongue-in-cheek comment.  I am sorry that
you took it too seriously.

>   This patch won't give any performance gain, but it would force
>   people to keep the array in order :-)

That is exactly why I discarded what I did the other day.  Without an
active mechanism to force the orderedness, such a change simply introduces
a downside of letting a mistake go unnoticed, without any real upside (as
you measured and saw no performance gain).

A better project hygine is a good thing to aim for, and I would imagine
that you could add "--verify-builtin-command-table" as an unadvertised
option to "git" wrapper, and make t/t0000-basic.sh call it to minimize the
downside risk.  But without such an active measure to prevent mistakes, we
would be relying on somebody getting caught on a ticking bomb and
reporting it, which is not a good tradeoff between risk and reward.

--
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


[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]