Re: [PATCH v4 2/2] Doc/check-ref-format: clarify information about @{-N} syntax

[Kaartic Sivaraam <kaartic.sivaraam@gmail.com>]

On Thursday 14 December 2017 11:32 PM, Junio C Hamano wrote:
> Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:
> 
> Looks alright.
> 
> It was made unnecessarily harder to review because it was marked as
> 2/2, even though this no longer applies on top of the copy of 1/2
> that was merged some time ago.

Sorry about that but I don't remember doing anything that made it not to 
apply on top of 1/2. (I just amended my changes to my topic branch. It 
can be found at [1])


> I needed to find that it was rebased
> on top of 'master';

I don't remember doing any rebase on top of 'master'. My topic was (and 
still is) based on the 'master' when it was pointing at 89ea799ff (Sync 
with maint, 2017-11-15). Anyways, it's my mistake as I didn't specify 
the branch on which I based this. Sorry about that.


> 
> Also re-wrapping the lines only to squeeze in "but be cautious..."
> and replace s/branch/checkout/ in a few places did not help to make
> it easy to spot what's changed.
> 

I expected this would happen but I thought the line shouldn't grew too 
much so that they have to re-wrapped. Seems it would have been better if 
I did the re-wrapping as a follow-up commit (didn't strike me then).


> This part looked a bit strange.
> 
>> +it can be used as a valid branch name e.g. when creating a new branch
>> +(but be cautious when using the previous checkout syntax; it may refer
>> +to a detached HEAD state). The rule `git check-ref-format --branch
> 
> I think "e.g. when creating a new branch" is a parenthetical remark,
> so it should be inside parenthesis.

It was. I brought them out to introduce the parenthetical warning. I'll 
send a v5 by putting the remark back inside parantheses and bringing the 
warning out. If it's not ok, let me know. I'll also try to do the 
re-wrapping as a separate cleanup patch.


>  As the last three lines in the
> new text (quoted above) already warns that it may not be a branch name,
> I am not sure if the "but be cautious" adds much value, though.
> 

That warning was for the impatient readers, who might want to find quick 
answers as to why they saw an odd behaviour (check-ref-froamt --branch 
not failing for a commit object name) (or) those who would want to use 
'check-ref-format --branch' but do not find time to read the whole 
paragraph.