serf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ivan Zhakov <i...@visualsvn.com>
Subject Re: [serf-dev] [serf] r2489 committed - In preparation of serf 1.4.0, remove the get_remaining function from t...
Date Mon, 07 Sep 2015 07:35:59 GMT
On 7 September 2015 at 00:25, Lieven Govaerts <lgo@mobsol.be> wrote:
> Hey Ivan,
>
> On Mon, Aug 31, 2015 at 3:20 PM, Ivan Zhakov <ivan@visualsvn.com> wrote:
>> On 6 April 2015 at 20:49, Ivan Zhakov <ivan@visualsvn.com> wrote:
>>> On 6 April 2015 at 19:02, Lieven Govaerts <lgo@mobsol.be> wrote:
>>>> On Mon, Apr 6, 2015 at 3:32 PM, Bert Huijben <bert@qqmail.nl> wrote:
>>>>>> -----Original Message-----
>>>>>> From: serf-dev@googlegroups.com [mailto:serf-dev@googlegroups.com]
On
>>>>>> Behalf Of serf@googlecode.com
>>>>>> Sent: maandag 6 april 2015 11:25
>>>>>> To: serf-dev@googlegroups.com
>>>>>> Subject: [serf-dev] [serf] r2489 committed - In preparation of serf
1.4.0,
>>>>>> remove the get_remaining function from t...
>>>>>>
>>>>>> Revision: 2489
>>>>>> Author:   lieven.govaerts
>>>>>> Date:     Mon Apr  6 09:24:18 2015 UTC
>>>>>> Log:      In preparation of serf 1.4.0, remove the get_remaining
function
>>>>>> from the
>>>>>> bucket API.
>>>>>>
>>>>>> This reverts most of r2008, r2009, r2010 and r2198. From r2008 I
kept the
>>>>>> read_bucket_v2 function, which is needed for set_config.
>>>>>
>>>>> If we still keep the read_bucket_v2 feature, what is the reason for just
removing get_remaining?
>>>>>
>>>>
>>>> I'm fixing all TODO's as discussed in the summer last year.
>>>> Get_remaining isn't finished yet and not used. So instead of waiting
>>>> until someone uses it, I'm removing it now so we can get 1.4 branched.
>>>>
>>>> We can still revert this revision after 1.4.x is branched though. In
>>>> fact, we can release 1.5.x with just this if an application wants to
>>>> make use of the (finalized) get_remaining feature.
>>>>
>>> What problem do we have with get_remaining() feature except
>>> read_bucket_v2() linkage problems? get_remaining() feature is not used
>>> in Subversion for only one reason: serf-trunk has version 2.0.0 so
>>> it's not possible to add version detection code.
>
> Huh? We always used a check on the next version of serf to check for
> trunk code, why is that not possible anymore?
> It's 2 years after you added the get_remaining API, and it's still not
> used in Subversion.
>
Current version in serf trunk is 2.0.0. I posted patch to use
get_remaining() API on Subversion list two years ago [1]. The only
reason it wasn't committed because serf-trunk has version 2.0.0
instead of 1.4.0, so I cannot complete my patch with correct API
check.

>>
>> I still don't understand your arguments on reverting get_remaining()
>> feature. Why you consider get_remaining() as isn't finished?
>
> I consider it as not finished because many bucket types don't have a
> get_remaining implementation, not even a default function, just NULL.
> If you use such a bucket in an aggregate bucket, you'll get a quite nasty crash.
>
Some buckets doesn't have get_remaining() implementation by design:
not every bucket type know data length. And
serf_bucket_get_remaining() returns SERF_LENGTH_UNKNOWN in this case.
Also I think it's better to report bug or fix it instead of reverting
feature from trunk especially without prior discussion.

>> The read_bucket_v2() linking problem also apply to your serf_config_t
>> feature, but we didn't reverted it from trunk.
>>
>> Also adding this feature latter will require read_bucket_v3() which
>> increase the mess.
>>
> My guess was then, and still is, that this feature can wait a release.
> I'm not against the feature, I'm against adding features that will not
> be used.
>
Of course we should not block release because of this feature, but you
reverted completed functionality from serf trunk with justification
that it's not used by one of serf user (Subversion). But how
Subversion should work if patch with use of serf_bucket_get_remaining
was committed to Subversion and released in 1.9.0 and then Serf
developers decide that serf_bucket_get_remaining() will not be
released in serf 1.4.0? Subversion cannot depends on serf
functionality that is not released, because serf developers may change
everything before release. Subversion cannot rely on unreleased serf
features.

[1] http://svn.haxx.se/dev/archive-2013-07/0133.shtml

-- 
Ivan Zhakov

Mime
View raw message