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

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Sep 7, 2016, 04:32 UTC
Message-ID: <8a13500c-55a0-a069-4b69-a651c218ded5@alum.mit.edu>
In-Reply-To: <236a95a0-59d4-3788-9104-5ca299119e66@gmail.com>

On 09/06/2016 04:25 PM, Jakub Narębski wrote:

Show 16 quoted lines

> 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