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

Reply via email to