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

Thiago Farina <tfransosi@gmail.com> writes:

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.