Re: [PATCH] commit: Add commit_list prefix to reduce_heads function.

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

 



Thiago Farina <tfransosi@xxxxxxxxx> writes:

> Signed-off-by: Thiago Farina <tfransosi@xxxxxxxxx>

I really do not like this.

The use of type "struct commit_list" to hold the set of parent commits is
incidental; if we had "struct commit_set", we would have written a
function with the same purpose, and named it the same "reduce_HEADS".

Adding commit_list to the name makes the code harder to read (and type)
with little added benefit.  "LIST"-ness is not the important part.

If a function takes a commit_list, named "reduce_HEADS", and returns a
commit_list, what it does should be obvious to you; otherwise you
shouldn't be touching the internal of git.

Having said that, I do not claim "reduce_heads" is the world most
wonderful short-sweet-descriptive name for what this function does and
there cannot be any better name.  But commit_list_reduce_head is not it.

If the patch were to rename the function, especially the HEADs part, to
clarify what it does, instead of how (iow, what type it uses to hold the
data) it does it, my reaction would have been very different.
--
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]