From: Jonathan Tan <jonathantanmy@google.com>
Date: Tue, 25 Oct 2022 18:00:17 GMT
Subject: Re: [PATCH v2 3/7] submodule--helper clone: create named branch
Message-ID: <20221025180017.977401-1-jonathantanmy@google.com>
In-Reply-To: <a4056e200eda9958efe77801b39bdebfdc1497a2.1666297239.git.gitgitgadget@gmail.com>

"Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes:
> Signed-off-by: Glen Choo <chooglen@google.com>

Add a note in the commit message that this will be used and tested in a 
subsequent patch. 

> -	if (argc || !clone_data.url || !clone_data.path || !*(clone_data.path))
> +	if (argc || !clone_data.url || !clone_data.path || !*(clone_data.path)
> +	    || (!!clone_data.branch != !!clone_data.branch_oid))
>  		usage_with_options(git_submodule_helper_usage,
>  				   module_clone_options);

I know that this is just internal code, but could we have a better 
diagnostic? You can leave the existing check alone, and then do the 
!!clone_data.branch != !!clone_data.branch_oid with a BUG() if the 
result is not what you expect.