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

[Pierre-Yves David]

On 05/08/2017 06:15 PM, Martin von Zweigbergk wrote:

On Mon, May 8, 2017 at 9:10 AM, Durham Goode  wrote:



On 5/6/17 6:55 AM, Martin von Zweigbergk wrote:




On May 5, 2017 23:57, "Pierre-Yves David"
>
wrote:



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

On Fri, May 5, 2017 at 11:44 AM, Pierre-Yves David
> wrote:



On 05/05/2017 08:38 PM, Martin von Zweigbergk wrote:


On Fri, May 5, 2017 at 11:35 AM, Pierre-Yves David
> wrote:




On 05/05/2017 06:41 PM, Martin von Zweigbergk wrote:



On Thu, May 4, 2017 at 11:26 PM, Pierre-Yves David
> wrote:



# HG changeset patch
# User Pierre-Yves David
>
# 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) 

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

[Pierre-Yves David]

On 05/06/2017 03:55 PM, Martin von Zweigbergk wrote:


On May 5, 2017 23:57, "Pierre-Yves David"
>
wrote:
 […]

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.


The point is not so much about how Facebook will adapt to the API 
change. They have a good continuous test coverage and many dev about to 
do the adjustment. My point is more than if we can find usage of that 
API in Facebook extension, it is a good hint that other extensions might 
do the same too.


It is not too important, I'll do a cleanup path on this in the next 
couple of weeks and align things one way or another (dropping the old 
one or adding some deprecwarn).


On 05/06/2017 08:20 PM, Gregory Szorc wrote:
> The changegroup APIs are horrible and are low-level. I favor deleting
> legacy ones that are no longer used in core. Extensions can test for
> function presence at run-time.

Yep! We are dropping that function and cleaning the API in all cases. We 
are just discussing keeping it around for a couple of month to help with 
third party extension.


--
Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

[Durham Goode]

On 5/6/17 6:55 AM, Martin von Zweigbergk wrote:



On May 5, 2017 23:57, "Pierre-Yves David"
>
wrote:



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

On Fri, May 5, 2017 at 11:44 AM, Pierre-Yves David
> wrote:



On 05/05/2017 08:38 PM, Martin von Zweigbergk wrote:


On Fri, May 5, 2017 at 11:35 AM, Pierre-Yves David
> wrote:




On 05/05/2017 06:41 PM, Martin von Zweigbergk wrote:



On Thu, May 4, 2017 at 11:26 PM, Pierre-Yves David
> wrote:



# HG changeset patch
# User Pierre-Yves David
>
# 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) 

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

[Gregory Szorc]

On Sat, May 6, 2017 at 11:22 AM, Augie Fackler  wrote:

>
> > On May 6, 2017, at 2:20 PM, Gregory Szorc 
> wrote:
> >
> >>> 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)
> >>
> > The changegroup APIs are horrible and are low-level. I favor deleting
> legacy ones that are no longer used in core. Extensions can test for
> function presence at run-time.
>
> Horrible in what way? Data loss, or just awkward?


Awkward. Remember the long series I sent last year to refactor the entire
thing to inject some sanity? It was queued then unqueued, which is why the
API is still horrible (although slightly less bad with this series). There
is a bunch of redundancy and it isn't clear what the purpose of each
function is.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

[Augie Fackler]

> On May 6, 2017, at 2:20 PM, Gregory Szorc  wrote:
> 
>>> 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)
>> 
> The changegroup APIs are horrible and are low-level. I favor deleting legacy 
> ones that are no longer used in core. Extensions can test for function 
> presence at run-time.

Horrible in what way? Data loss, or just awkward?
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

[Gregory Szorc]

On Fri, May 5, 2017 at 11:44 AM, Pierre-Yves David <
pierre-yves.da...@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
>>  wrote:
>>
>>>
>>>
>>> On 05/05/2017 06:41 PM, Martin von Zweigbergk wrote:
>>>

 On Thu, May 4, 2017 at 11:26 PM, Pierre-Yves David
  wrote:

>
> # HG changeset patch
> # User Pierre-Yves David 
> # 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)
>

The changegroup APIs are horrible and are low-level. I favor deleting
legacy ones that are no longer used in core. Extensions can test for
function presence at run-time.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

[Martin von Zweigbergk via Mercurial-devel]

On May 5, 2017 23:57, "Pierre-Yves David" 
wrote:



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

> On Fri, May 5, 2017 at 11:44 AM, Pierre-Yves David
>  wrote:
>
>>
>>
>> On 05/05/2017 08:38 PM, Martin von Zweigbergk wrote:
>>
>>>
>>> On Fri, May 5, 2017 at 11:35 AM, Pierre-Yves David
>>>  wrote:
>>>



 On 05/05/2017 06:41 PM, Martin von Zweigbergk wrote:

>
>
> On Thu, May 4, 2017 at 11:26 PM, Pierre-Yves David
>  wrote:
>
>>
>>
>> # HG changeset patch
>> # User Pierre-Yves David 
>> # 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
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

[Augie Fackler]

On Fri, May 05, 2017 at 12:16:50PM -0700, Martin von Zweigbergk via 
Mercurial-devel wrote:
> On Fri, May 5, 2017 at 11:44 AM, Pierre-Yves David
>  wrote:
> >
> >
> > On 05/05/2017 08:38 PM, Martin von Zweigbergk wrote:
> >>
> >> On Fri, May 5, 2017 at 11:35 AM, Pierre-Yves David
> >>  wrote:
> >>>
> >>>
> >>>
> >>> On 05/05/2017 06:41 PM, Martin von Zweigbergk wrote:
> 
> 
>  On Thu, May 4, 2017 at 11:26 PM, Pierre-Yves David
>   wrote:
> >
> >
> > # HG changeset patch
> > # User Pierre-Yves David 
> > # 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.

FWIW, my gut feeling is that few-to-no extensions are likely to
generate bundles. I've done lots of awful invasive stuff in extensions
and never actually generated bundles as part of them.

(I don't feel strongly about preserving compat either, but my feeling
on API stability is that our warnings should be seen by extension
authors as best-effort and not reliable.)

>
> >
> > Cheers,
> >
> > --
> > Pierre-Yves David
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

[Martin von Zweigbergk via Mercurial-devel]

On Fri, May 5, 2017 at 11:44 AM, Pierre-Yves David
 wrote:
>
>
> On 05/05/2017 08:38 PM, Martin von Zweigbergk wrote:
>>
>> On Fri, May 5, 2017 at 11:35 AM, Pierre-Yves David
>>  wrote:
>>>
>>>
>>>
>>> On 05/05/2017 06:41 PM, Martin von Zweigbergk wrote:


 On Thu, May 4, 2017 at 11:26 PM, Pierre-Yves David
  wrote:
>
>
> # HG changeset patch
> # User Pierre-Yves David 
> # 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.

>
> Cheers,
>
> --
> Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

[Pierre-Yves David]

On 05/05/2017 08:38 PM, Martin von Zweigbergk wrote:

On Fri, May 5, 2017 at 11:35 AM, Pierre-Yves David
 wrote:



On 05/05/2017 06:41 PM, Martin von Zweigbergk wrote:


On Thu, May 4, 2017 at 11:26 PM, Pierre-Yves David
 wrote:


# HG changeset patch
# User Pierre-Yves David 
# 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)

Cheers,

--
Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

[Martin von Zweigbergk via Mercurial-devel]

On Fri, May 5, 2017 at 11:35 AM, Pierre-Yves David
 wrote:
>
>
> On 05/05/2017 06:41 PM, Martin von Zweigbergk wrote:
>>
>> On Thu, May 4, 2017 at 11:26 PM, Pierre-Yves David
>>  wrote:
>>>
>>> # HG changeset patch
>>> # User Pierre-Yves David 
>>> # 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?

>
> The function are dropped in the version released after their deprecation (cf
> recent localrepo API cleanup and vfs movement).
>
> We have been using this approach for a couple of year and this has provent
> extremely useful.
>
> Cheers,
>
> --
> Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

[Pierre-Yves David]

On 05/05/2017 06:41 PM, Martin von Zweigbergk wrote:

On Thu, May 4, 2017 at 11:26 PM, Pierre-Yves David
 wrote:

# HG changeset patch
# User Pierre-Yves David 
# 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.


The function are dropped in the version released after their deprecation 
(cf recent localrepo API cleanup and vfs movement).


We have been using this approach for a couple of year and this has 
provent extremely useful.


Cheers,

--
Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

[Martin von Zweigbergk via Mercurial-devel]

On Thu, May 4, 2017 at 11:26 PM, Pierre-Yves David
 wrote:
> # HG changeset patch
> # User Pierre-Yves David 
> # 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?
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

[Pierre-Yves David]

# HG changeset patch
# User Pierre-Yves David 
# 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.

diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
--- a/mercurial/changegroup.py
+++ b/mercurial/changegroup.py
@@ -976,8 +976,10 @@ def getchangegroup(repo, source, outgoin
 bundler = getbundler(version, repo)
 return getsubset(repo, outgoing, bundler, source)
 
-# deprecate me once all users are gone
-getlocalchangegroup = getchangegroup
+def getlocalchangegroup(repo, *args, **kwargs):
+repo.ui.deprecwarn('getlocalchangegroup is deprecated, use getchangegroup',
+   '4.3')
+return getchangegroup(repo, *args, **kwargs)
 
 def changegroup(repo, basenodes, source):
 # to avoid a race we use changegroupsubset() (issue1320)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel