Re: [RFC PATCH 3/5] branch: cleanup branch name validation

From: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
Date: Sep 20, 2017, 12:04 UTC
Message-ID: <1d620d52-5326-269a-8710-160b75fada81@gmail.com>
In-Reply-To: <xmqqzi9qngq9.fsf@gitster.mtv.corp.google.com>

On Wednesday 20 September 2017 09:50 AM, Junio C Hamano wrote:

Show 9 quoted lines

> Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:
>
>> -int validate_new_branchname(const char *name, struct strbuf *ref,
>> -			    int force, int attr_only)
>> +int validate_branch_update(const char *name, struct strbuf *ref,
>> +			   int could_exist, int clobber_head)
> "update" to me means something already exists and the caller is
> asking this function if it is OK to update it, but is that what this
> function is used for?

Of course not. I couldn't come up with better names.

>   I do not find the original name too bad, but
> if I were renaming it, I'd call it ok_to_create_branch(), with the
> understanding that forcing a recreation of an existing branch falls
> into the wider definition of "create".

Thanks for giving a better alternative. Sounds catchy. How about `validate_branch_creation`? For some unknown reason, I seem to like to have the word "validate" in the name. If that's not ok, I'll use the suggested name.

Show 7 quoted lines

> Also I'd avoid "could", which can be taken as an optimization hint
> (i.e. "you usually do not have to worry about this thing to already
> exist, but I am telling you that for this one call that is not the
> case and you need to be a bit more careful by spending extra cycles
> to see if it is and deal with the situation accordingly if it indeed
> is"), and use "ok" as part of the name for the parameter (or flip
> the meaning of it and say "create_only" or something).

Will fix that.

--- Kaartic