On 2019-10-28 at 02:56:14, Junio C Hamano wrote:
> > +short_oid () {
> > +	local oid=$(git hash-object "$1") &&
> > +	git rev-parse --short "$oid"
> > +}
> 
> Given that we do not use the former helper to compute a regular
> file object name without having a concrete file on the filesystem,
> I do not mind calling it symlink_oid at all.  But then this may want
> to be called file_oid to make the contrast better, and it would
> match the use of these two in a test near the end of this patch.

Yeah, I can make that change.

> >  test_expect_success 'diff new symlink and file' '
> > -	cat >expected <<-\EOF &&
> > +	symlink=$(symlink_oid xyzzy) &&
> > +	cat >expected <<-EOF &&
> >  	diff --git a/frotz b/frotz
> >  	new file mode 120000
> > -	index 0000000..7c465af
> > +	index 0000000..$symlink
> 
> It is a mistake to name the variable "symlink", even though
> symlink_oid is a good name for this helper function.  
> 
> If you were showing a change that updates the symlink RelNotes from
> pointing at Documentation/RelNotes/2.0.0.txt to
> Documentation/RelNotes/2.1.0.txt, for example, you would say
> 
> 	old=$(symlink_oid Documentation/RelNotes/2.0.0.txt) &&
> 	new=$(symlink_oid Documentation/RelNotes/2.1.0.txt) &&
> 	cat expect <<-EOF &&
> 	diff --git a/RelNotes b/RelNotes
> 	index $old..$new
> 	...
> 	EOF
> 
> so I'd expect $new (on the right hand side of ..) would read more
> natural.

Can change.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204