Re: [PATCH v2 02/38] rename_ref_available(): add docstring

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

 



On 09/06/2016 04:25 PM, Jakub Narębski wrote:
> W dniu 04.09.2016 o 18:08, Michael Haggerty pisze:
> 
>> +/*
>> + * Check whether an attempt to rename old_refname to new_refname would
>> + * cause a D/F conflict with any existing reference (other than
>> + * possibly old_refname). If there would be a conflict, emit an error
>> + * message and return false; otherwise, return true.
>> + *
>> + * Note that this function is not safe against all races with other
>> + * processes (though rename_ref() catches some races that might get by
>> + * this check).
>> + */
>> +int rename_ref_available(const char *old_refname, const char *new_refname);
> 
> Just a sidenote: does Git have a naming convention for query functions
> returning a boolean, for example using is_* as a prefix?

I've never heard of an official convention like that, and don't see it
documented anywhere. But there are a lot of functions (and variables)
whose names start with `is_`, and it seems like a reasonable idea.

> That is, shouldn't it be
> 
>   int is_rename_ref_available(const char *old_refname, const char *new_refname);

I agree, that would be a better name.

But that naming change is orthogonal to this patch series, which only
adds a docstring to the function. I don't think it's worth rerolling
this 38-patch series to add it. So I suggest that we keep your idea in
mind for the next time this code is touched (or feel free to submit a
patch yourself, preferably on top of this patch series to avoid conflicts).

Thanks,
Michael




[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]