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

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

W dniu 04.09.2016 o 18:08, Michael Haggerty pisze:

quoted

+/*
+ * 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