On Thursday 14 December 2017 11:32 PM, Junio C Hamano wrote:
Kaartic Sivaraam <kaartic.sivar...@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.

Reply via email to