Re: [PATCH] commit: Append commit_list prefix in two function names.

From: Thiago Farina <tfransosi@gmail.com>
Date: Nov 27, 2010, 01:54 UTC
Message-ID: <AANLkTi=V7e-KFhKVDLjH4TvoT6U3xmFieo5uqigPhqKF@mail.gmail.com>
In-Reply-To: <20101114211905.GB10150@burratino>

On Sun, Nov 14, 2010 at 7:19 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 7 quoted lines

> Thiago Farina wrote:
>
>> Append commit_lis prefix to insert_by_date function and to sort_by_date,
>> so it's clear that these functions refer to commit_list structure.
>
> Nits: by "Append" I think you mean "Add"; missing "t" after commit_lis.
>

Thanks, I will fix this in a follow up.

Show 18 quoted lines

> This gives the oft-used insert_by_name() function a fairly long name:
>
>        commit_list_insert_by_name
>
> The proposed name is long enough to be unwieldly.  It might have
> the virtue of fitting better with some of the commit_list lib:
>
>        commit_list_count
>        commit_list_insert
>        free_commit_list
>
> Compare:
>
>        sort_by_date
>        pop_most_recent_commit
>        sort_in_topological_order
>        pop_commit
>

I don't understand what you are arguing here. Is about the size of "commit_list_insert_by_name"? I don't care about it's size, I just want to make it consistent by adding commit_list in the functions that are part of the commit_list API.

> Perhaps this change would work better if some of the others were
> renamed at the same time?
>

I don't think so, this would increasing the size of the change and make it less readable.