Re: [PATCH] Use 'fast-forward' all over the place

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Oct 25, 2009, 08:56 UTC
Message-ID: <94a0d4530910250156h1dfe345by51581b2e8acd6629@mail.gmail.com>
In-Reply-To: <7v3a5860gr.fsf@alter.siamese.dyndns.org>

On Sat, Oct 24, 2009 at 10:44 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 13 quoted lines

> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>>> I suspect the patch would have been much easier to the reviewers it it
>>> stated somewhere in the log message:
>>>
>>>  (1) how the mechanical change was produced;
>>
>> There wasn't such.
>
> That is actually a bad news; it is even worse than mechanical spotting
> followed by manual inspection.  It would force us feel _more_ worried, as
> we would then need to grep for leftovers that your manual conversion may
> not have caught.  Sigh...

Perhaps next time I'll do one round manual, and another one automatic, but in this case I didn't think it was so difficult to review hunk by hunk.

Show 14 quoted lines

>>>  (2) what criteria was used to choose between leaving the mechanical
>>>     change as-is and rewording them manually; and
>>
>> If it wasn't straight forward. I considered the following straightforward:
>> fast forward -> fast-forward
>> fast forwarded -> fast-forwarded
>> fast forwarding -> fast-forwarding
>> fast forwardable -> fast-forwardable
>> non-fast forward -> non-fast-forward
>> Fast forward -> Fast-forward
>> Fast forwarding -> Fast-forwarding
>
> All of these are what "s/([fF])ast forward/$1ast-forward/g" does, aren't
> they?

I guess, yes. But that's not how I did it, so I couldn't be sure.

Show 14 quoted lines

>> I couldn't parse that. From what I can see "Fast forward" was
>> emphasized because the author thought the words didn't make much sense
>> separated. Now that the word is fast-forward, there's no need to
>> emphasize.
>
> Even after your patch, hunk beginning on line 1384 of the
> user-manual says
>
>    ... then git just performs a "fast-forward"; the head of the ...
>
> and I think you did the right thing by keeping these dq here.  This is the
> first occurrence of the word followed by its explanation and for that
> reason, the word deserves to be emphasized---IOW, the context calls for an
> emphasis.

Yes, exactly.

Show 5 quoted lines

> In the "Important note!" part, we talk about the pull operation that
> normally creates a merge commit, and _in contrast_, under a particular
> condition (namely, "no local changes"), it does something different
> (namely, a "fast-forward").  We should keep the emphasis on "fast-forward"
> here for exactly the same reason---the context calls for an emphasis

I don't think so. The emphasis in this case breaks the readability of the text for no reason: with no local changes git will simply do a fast-forward merge

Can be perfectly understood as it is. But in any case, that's out of the scope of this patch.

Cheers.

-- 
Felipe Contreras