From: Jeff Hostetler <git@jeffhostetler.com>
Date: Fri, 22 Apr 2022 20:47:09 GMT
Subject: Re: [PATCH v5 23/28] fsmonitor: never set CE_FSMONITOR_VALID on submodules
Message-ID: <7cbabd84-0c4d-5517-0d5d-3f5725d3105d@jeffhostetler.com>
In-Reply-To: <xmqqv8v3l3w1.fsf@gitster.g>



On 4/20/22 7:41 PM, Junio C Hamano wrote:
> "Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> +create_super () {
>> +	super=$1 &&
>> +
>> +	git init "${super}" &&
> 
> It is not wrong per-se, but a simple reference to a shell variable
> without magic interpolation like ${parameter-word} is easier to read
> without {} around the variable name, i.e.
> 
> 	git init "$super"
> 
> an exception of course is when you want to suffix its value with
> alnum, i.e.
> 
> 	for d in "$super" "${super}1" "$super"2
> 	do
> 		...
> 
> and writing it as "${super}1" would probably be easier to see what
> is going on than "$super"2 notation.
> 
>> +	echo x >${super}/file_1 &&
>> +	echo y >${super}/file_2 &&
>> +	echo z >${super}/file_3 &&
> 
> CodingGuidelines still says that these redirection targets with
> variable interpolation must be enclosed in double-quotes, i.e.
> 
> 	echo x >"$super/file_1" &&
> 
>> +	mkdir ${super}/dir_1 &&
> 
> The double quotes around "${super}" we saw on "git init" indicates
> that the helper function wants to be prepared to handle a directory
> path with possibly $IFS whitespace characters in it correctly, so
> let's make sure we are consistently prepared for such a parameter,
> i.e.
> 
> 	mkdir "$super/dir_1" &&
> 
> The same applies to the rest of the script.
> 
> Thanks.
> 

good points.  i'll fixup and resend.
thanks!

jeff