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

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

 



Thiago Farina wrote:
> On Sun, Nov 14, 2010 at 7:19 PM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:

>> 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,

For code clarity, length of function names can matter...

> I just
> want to make it consistent by adding commit_list in the functions that
> are part of the commit_list API.

... though this consideration is probably more important.

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

Even if split up into multiple patches?  I don't think it makes much
sense to say "functions in the commit_list API all start with
commit_list_" while at the same time leaving half of the functions in
the commit_list API without that suffix.

By the way, how did this come up?  Presumably some particular code
was confusing?  If so, that information could be useful as an example
in the log message.

Regards,
Jonathan
--
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]