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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]