On May 5, 2017 23:57, "Pierre-Yves David" <pierre-yves.da...@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.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 >>> <pierre-yves.da...@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.da...@ens-lyon.org> wrote: >>>>> >>>>>> >>>>>> >>>>>> # HG changeset patch >>>>>> # User Pierre-Yves David <pierre-yves.da...@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
_______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel