Re: [PATCH 1/5] Introduces for_each_revision() helper

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

 



Junio C Hamano <junkio@xxxxxxx> wrote:
> The reason I do not like this particular one is because both
> operations you are hiding are not simple operations like
> "initialize a variable to list head" or "follow a single pointer
> in the structure", but rather heavyweight operations with rather
> complex semantics.  I would want to make sure that people
> realize they are calling something heavyweight when they use the
> revision traversal.

But in_merge_base is heavyweight if the two commits are in the
same object database, but aren't connected at all.  You'll need
to traverse both histories before aborting and saying there is
no merge base.  That ain't cheap on large trees.  But its also a
single line of code.

Anyway, my original problem with this macro was the way it was
defined.  I think Luiz was able to fix most of my issues with it
in his latest version, but I still have a personal distaste for
hiding things like a for(;;) construct in a macro, or allowing a
macro parameter to be used more than once within the definition of
the macro (unexpected side-effects of evaluating an more than once).

-- 
Shawn.
-
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]