Re: [PATCH] commit: Add commit_list prefix to reduce_heads function.
- From
Thiago Farina <tfransosi@gmail.com>
- Date
- Dec 5, 2010, 21:29 UTC
- Message-ID
- <AANLkTi=QK=N+_iGR9-47JKFs_SDKujJ8c4mtnnM0yo94@mail.gmail.com>
- In-Reply-To
- <7vk4joszcj.fsf@alter.siamese.dyndns.org>
On Sun, Dec 5, 2010 at 7:07 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 18 quoted lines
> Thiago Farina <tfransosi@gmail.com> writes: > >> Also I thought that as Junio already picked up the other patch. It's >> was a hint that the other functions that has "struct commit_list *l" >> as its parameters could be renamed as well. > > You took a wrong hint and I think that is because you didn't think about > what naming is for. > > "insert-by-date" does not say _why_ you want things to be inserted by date > (neither "sort-by-date"). They are pretty generic looking names for any > function that deal with a list of elements that record date. It makes > sense to anticipate there will be many other such functions that deal with > different kinds of lists that hold date-recording things, and naming one > of them "this deals with list of COMMITS" by saying "commit_list_foo" > makes quite a lot of sense, as "insert-by-date" does not give sufficient > information to the reader. >
That makes sense to me. And clarified why the complain at all. And you are right.
Would these be a candidates for adding commit_list_ prefix?
free_commit_list -> commit_list_free contains pop_most_recent_commit -> I'm not sure about this because of the length of it, as Jonathan pointed in this thread. pop_commit