Re: [PATCH 4/4] fast-import: only store commit objects

[Felipe Contreras <felipe.contreras@gmail.com>]

On Fri, May 3, 2013 at 6:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>>> A safe and sane approach may be to teach these an option to tell
>>> them to omit non-commits or to emit all kinds, and make remote-bzr
>>> use that to exclude non-commits.
>>
>> This has nothing to do with remote-bzr, or any remote helper. These
>> objects are not useful, not even to 'git fast-export'.
>>
>>> If the defaults is matched to the
>>> current behaviour, nobody will get hurt
>>
>> Changing nothing always ensures that nobody will get hurt, but that
>> doesn't improve anything either.
>
> I do not quite follow.  Allowing existing code to depend on old
> behaviour, while letting new code that knows it does not need
> anything but commits, will get the best of both worlds. The new code
> will definitely improve (otherwise you won't be writing these
> patches), and the old code will not get hurt. Where is this "doesn't
> improve anything" coming from?

So let's suppose we add a new 'feature no-blob-store' to 'git
fast-import', and some clients of 'git fast-import' enable it, and
benefit from it.

How does that benefit the rest of fast-import clients? You are
worrying about clients that most likely don't exist, and don't let
existing real clients benefit for free. This is like premature
optimization.

But whatever, let's keep writing and discarding these blobs, at least
the previous patches do make such operations fast.

You can drop this patch, because I'm not going to write code for
clients that don't exist. And it seems clear that even if I
investigate client apps of 'git fast-import' and I'm unable to find a
single one that utilizes blobs, you still wouldn't apply this patch.
So there's no point in investigating what are the *actual* users, if
all we are every going to do is worry about hypothetical ones.

My main interest is the previous patches, if anybody has any issues
with the way this patch is handled, they can either work on the patch
itself, or start a new thread in which I will not participate, I will
rather concentrate on issues that affect *real* users, and have real
fixes reachable today, and the previous patches in this series fit
that bill.

Cheers.

-- 
Felipe Contreras