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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 24, 2009, 19:44 UTC
Message-ID: <7v3a5860gr.fsf@alter.siamese.dyndns.org>
In-Reply-To: <94a0d4530910241212s5d644eb6u66c6f96feafcaf10@mail.gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

Show 6 quoted lines

>> 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...

Show 11 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 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.

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 was about to read the rest of the patch (the non-doc part) but instead I ended up spending another 20 minutes writing this message, so the review by me on the remainder has to wait til sometime later.