[PATCH 5 of 7] changegroup: deprecate 'getlocalchangroup' (API)

[Martin von Zweigbergk]

On May 5, 2017 23:57, "Pierre-Yves David" <pierre-yves.david at ens-lyon.org>
wrote:



On 05/05/2017 09:16 PM, Martin von Zweigbergk wrote:

> On Fri, May 5, 2017 at 11:44 AM, Pierre-Yves David
> <pierre-yves.david at ens-lyon.org> wrote:
>
>>
>>
>> On 05/05/2017 08:38 PM, Martin von Zweigbergk wrote:
>>
>>>
>>> On Fri, May 5, 2017 at 11:35 AM, Pierre-Yves David
>>> <pierre-yves.david at ens-lyon.org> wrote:
>>>
>>>>
>>>>
>>>>
>>>> On 05/05/2017 06:41 PM, Martin von Zweigbergk wrote:
>>>>
>>>>>
>>>>>
>>>>> On Thu, May 4, 2017 at 11:26 PM, Pierre-Yves David
>>>>> <pierre-yves.david at ens-lyon.org> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> # HG changeset patch
>>>>>> # User Pierre-Yves David <pierre-yves.david at octobus.net>
>>>>>> # Date 1493894621 -7200
>>>>>> #      Thu May 04 12:43:41 2017 +0200
>>>>>> # Node ID 2f51cfeac5bcf8ee266a6fada56517d5d44d9b6b
>>>>>> # Parent  1e8427b7d0b9ce66c5ba34c2cdb64821ff909267
>>>>>> # EXP-Topic changegroup.cleanup
>>>>>> # Available At
>>>>>> https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>>>>>> #              hg pull
>>>>>> https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r
>>>>>> 2f51cfeac5bc
>>>>>> changegroup: deprecate 'getlocalchangroup' (API)
>>>>>>
>>>>>> We have 'getchangegroup' with a shorter name for the exactly same
>>>>>> feature. Now
>>>>>> that all users are gone we can formally deprecate it.
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> We usually just delete methods that we no longer use internally. Why
>>>>> not do that here?
>>>>>
>>>>
>>>>
>>>>
>>>> When function are likely to be used by extension we try to avoid
>>>> dropping
>>>> them and we deprecated them for a version. This helps third party
>>>> extensions
>>>> to smoothly detect the API changes (usually through test run) and
>>>> smoothly
>>>> upgrade their code over the version cycle.
>>>>
>>>
>>>
>>> In this case I suspect it won't help many of them because I probably
>>> broke most or all anyway with
>>> https://www.mercurial-scm.org/repo/hg/rev/282b288aa20c. I thought we
>>> were okay with that kind of changes. Are you saying I should have
>>> instead added duplicate methods with the new signatures and only
>>> deprecated the existing methods?
>>>
>>
>>
>> Generating changegroup is a quite core feature in Mercurial. I suspect
>> their
>> are extension out there using these and I tried to be careful. That is a
>> gut
>> feeling. I'm did not looked at actual data. (that gut feeling proved right
>> for "vfs", but might be wrong here)
>>
>> (so, yes I would have been more careful with your API change too)
>>
>
> Well, given that my patch probably broke most of those extensions, I
> don't see much reason for your series to be more careful. But there's
> little harm in doing it, so I'll queue it once tests have run etc.
>

I agree doing both approach at the same time is sub-optimal :-)
We should either restore some of the (deprecated) compatibility dropped
your series or remove the function deprecated in mine.

I've digged up a bit and  already found a couple of Facebook extensions
using getlocalchangegroup so I think it is safer to assume there are
external users.


I don't care much. Durham, do you guys care enough so we should back out
the change. I don't care enough to clean up that I'm going to bother with
deprecation.


Cheers,

-- 
Pierre-Yves David
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170506/74dd5b05/attachment.html>