Subject: Re: [PATCH v2 2/6] builtin/am: make sure state files are text

From: Junio C Hamano <[email-masked]>
Date: Tue, 25 Aug 2015 09:19:13 -0700

Jeff King <[email-masked]> writes:

> On Mon, Aug 24, 2015 at 01:58:06PM -0700, Junio C Hamano wrote:
>
>> We forgot to terminate the payload given to write_file() with LF,
>> resulting in files that end with an incomplete line.  Teach the
>> wrappers builtin/am uses to make sure it adds LF at the end as
>> necessary.
>
> Is it even worth doing this step? It's completely reverted later in the
> series. I understand that we do not want to hold the fix to git-am
> hostage to write_file refactoring, but I don't see any reason these
> cannot all graduate as part of the same topic.
>
> Ignore me if you really are planning on doing the first two to "maint"
> and holding the others back for "master".

Not really.  The primary reason why this step exists and 1-2/6 make
a sufficient fix by themselves is because I wasn't even sure if
3-6/6 were worth doing.

As to "flags exposed to callers" vs "with and without gently", when
we change the system to allow new modes of operations (e.g. somebody
wants to write a binary file, or allocate more flag bits for their
special case), I'd expect that we'd add a more general and verbose
"write_file_with_options(path, flags, fmt, ...)"), gain experience
with that function, and then possibly introduce canned thin wrappers
(e.g. write_binary_file() that is a synonym to passing BINARY but
not GENTLY) if the new thing proves widely useful, just like I left
write_file() and write_file_gently() in as fairly common things to
do.