On 3 Jan 2017, at 22:40 , Guillermo Polito <guillermopolito@gmail.com> wrote:

On Tue, Jan 3, 2017 at 8:56 PM, Eliot Miranda <eliot.miranda@gmail.com> wrote:

On Tue, Jan 3, 2017 at 2:24 AM, Henrik Johansen <henrik.s.johansen@veloxit.no> wrote:
It was my impression there was a procedure in place earlier for moving methods from SmalltalkImage to classes with a more singular responsibility that ensures a smooth upgrade experience;

Yes, but it depends mostly on the user and there is no automated check for it...
So it happens that people (myself also) sometimes remove stuff without going into the deprecation phase... :/

1) create accessor method on Smalltalk (if none of the existing are appropriate)
2) implement the old methods on new class in backwards compatability category
3) Deprecate method on SmalltalkImage, on the form:
doX: someThing
self deprecated: 'Use Smalltalk specializedArea x: someThing instead' on: 'foo' in: 'bar'.
^self specializedArea x: someThing

(Ref: #os, #vm)

I don't think step 1 is mandatory. Actually I would say it should be forbidden.
Why should Smalltalk be a facade for every library in the system?

I agree however that for library/method rewrites we should at least keep the old interface if possible. Now, when it happens that the new API is not compatible with the old one, only the migration should be nicely documented...

Not every library, and definitely not as the default for any new functionality, but (imho) at least for preexisting functionality on SmalltalkImage that is moved, ref the original discussion:

http://forum.world.st/SmalltalkImage-current-vs-Smalltalk-td1574575i20.html

I've run into a few cases where this is not the case when moving to Pharo 5, should they be changed, or am I wrong, and the procedure above is no longer considered valuable?

Case 1:
SmalltalkImage removeFromShutDownList: aClass (and removeFromStartUpList:)
self deprecated:
'Please use registration methods provided in SessionManager / registration category.',
String cr,
'ex: SessionManager default unregisterClassNamed: self name'.

There's a new #session accessor on SmalltalkImage, but...
1) SessionManager does not implement a backwards compatible protocol.

I'd say that's life...

2) The deprecated message above is wrong, the Smalltalk session unregisterClassNamed: is preferrable to referencing the class directly.

And that this requires a fix om the docs.

3) The deprecated method does nothing, rather than redirect to the new place. Silent errors when deprecation warnings are turned of, yay.

Well redirection should be done wether it is possible.
But if it is not, then the method should be cancelled with an exception? Like that even if the deprecation is ignored, the error does not go silent.

I guess the point I was trying to make; even if it's rare to unregister from startup/shutdown individually, and the new recommended approach is to use unregisterClass: to do both, when you replace such core functionality, you *need* to provide a backwards compatible protocol as well (especially since there's no 1-1 mapping between existing and previous behaviour), not just refer to unregisterClass: and let the old usage (potentially) silently do nothing.

When there's still a caller in the *base* image (#deinstall in InputEventFetcher), you know the frequency by which this code is actually called/tested.

Sampling the Pharo5 version of a major package like Seaside, the included GrPharoPlatform still does

removeFromStartUpList: anObject

"Remove anObject from the startup list in the system."

Smalltalk removeFromStartUpList: anObject

Would it be tested in time for the version where the deprecated method is actually removed? No one knows.

But in the mean time, trying to remove Seaside, or any other existing package which tries to play nice and unregister itself, will not clean up correctly when you press proceed, as is expected when you encounter a deprecation warning. The less nice alternative, is indeed to raise an exception after the deprecation, indicating this *must* be rewritten now to continue working as intended.

4) The deprecation message uses the deprecated form which does not include the parameters needed to know when to expect it removed forever.

Did not get this one

deprecated: tells you nothing about when, and in which version, the method was considered obsolete.

By using the expanded deprecated:on:in:, it's easier to tell when it should be expected to be gone for good (for instance, if it says it was deprecated in: 'Pharo5', you'd expect to have to migrate code by the time Pharo7 rolls around)

http://forum.world.st/what-about-deprecating-deprecated-td2306905.html

Also, the corresponding addTo... methods have not been deprecated, but redirect to SessionManager directly rather than through #session...

Case 2:
EndianDetector: Used to be Smalltalk #endianness

1) The method on SmalltalkImage has simply been removed, no deprecation.

I agree we should've deprecated.
Maybe we can deprecate it now in Pharo6 for Pharo7 (before removing the methods from Smalltalk definitively?)

Not backporting such a missing deprecation to Pharo5 would sort of defeat the purpose of using deprecations to ensure a smooth upgrade experience, no?