Re: [PATCH 3/6] refs.c: replace write_str_in_full by write_in_full

From: Stefan Beller <sbeller@google.com>
To: Jeff King <peff@peff.net>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>,
	Michael Haggerty <mhagger@alum.mit.edu>,
	Loic Dachary <loic@dachary.org>
Subject: Re: [PATCH 3/6] refs.c: replace write_str_in_full by write_in_full
Date: Wed, 21 Jan 2015 15:44:36 -0800	[thread overview]
Message-ID: <CAGZ79kaTUraVYc1Th0-8ajw4kMuU-Sir2e9dW1uy90CuQRzgeg@mail.gmail.com> (raw)
In-Reply-To: <20150121233843.GB11115@peff.net>

On Wed, Jan 21, 2015 at 3:38 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Jan 21, 2015 at 03:23:42PM -0800, Stefan Beller wrote:
>
>> There is another occurrence where we could have used write_str_in_full
>> (line 3107: write_in_full(lock->lk->fd, &term, 1)), so the current state
>> is inconsistent. This replaces the only occurrence of write_str_in_full
>> by write_in_full, so we only need to wrap write_in_full in the next patch.
>
> I had to read the first sentence a few times to figure out what you
> meant. But I am not sure it is even relevant. We do not care about the
> inconsistency.

You're not the first who needs to reread my stuff :/
I have the impression my English worsened since coming into the USA.

We do not care about the inconsistency, but we may care about the change itself:
"write_str_in_full is way better than write_in_full, so why the step backwards?"
And  I am trying to explain that this is not a huge step backwards but
rather improves
consistency.

> It is just "we are about to change how callers of
> write_in_full in this file behave, the wrapper gets in the way, and it
> does not add enough value by itself to merit making our future changes
> in two places".

That's actually true. Though that sounds as if we'd be lazy ("we only
want to make
one change, so let's bend over here")

I'll rethink the commit message.
Thanks,
Stefan