Deprecations on Smalltalk Image in Pharo 5

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Deprecations on Smalltalk Image in Pharo 5

Henrik Sperre Johansen
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;
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'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.
2) The deprecated message above is wrong, the Smalltalk session unregisterClassNamed: is preferrable to referencing the class directly.
3) The deprecated method does nothing, rather than redirect to the new place. Silent errors when deprecation warnings are turned of, yay.
4) The deprecation message uses the deprecated form which does not include the parameters needed to know when to expect it removed forever.

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.
2) Accessor remains the same, but all references in the base image has been rewritten to use EndianDetector directly...
2) Would not Smalltalk platform be a reasonable place to put this responsiblity, letting it use an EndianDetector behind the scenes? Both the preexisting #endianness, as well as the new #isLittleEndian and #isBigEndian?

Cheers,
Henry




Reply | Threaded
Open this post in threaded view
|

Re: Deprecations on Smalltalk Image in Pharo 5

Eliot Miranda-2


On Tue, Jan 3, 2017 at 2:24 AM, Henrik Johansen <[hidden email]> 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;
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'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.
2) The deprecated message above is wrong, the Smalltalk session unregisterClassNamed: is preferrable to referencing the class directly.
3) The deprecated method does nothing, rather than redirect to the new place. Silent errors when deprecation warnings are turned of, yay.
4) The deprecation message uses the deprecated form which does not include the parameters needed to know when to expect it removed forever.

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.
2) Accessor remains the same, but all references in the base image has been rewritten to use EndianDetector directly...
2) Would not Smalltalk platform be a reasonable place to put this responsiblity, letting it use an EndianDetector behind the scenes? Both the preexisting #endianness, as well as the new #isLittleEndian and #isBigEndian?

+1.  EndianDetector is a monstrosity.
 
Cheers,
Henry

_,,,^..^,,,_
best, Eliot
Reply | Threaded
Open this post in threaded view
|

Re: Deprecations on Smalltalk Image in Pharo 5

Guillermo Polito


On Tue, Jan 3, 2017 at 8:56 PM, Eliot Miranda <[hidden email]> wrote:


On Tue, Jan 3, 2017 at 2:24 AM, Henrik Johansen <[hidden email]> 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...


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.
 
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
 

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?)
 
2) Accessor remains the same, but all references in the base image has been rewritten to use EndianDetector directly...
2) Would not Smalltalk platform be a reasonable place to put this responsiblity, letting it use an EndianDetector behind the scenes? Both the preexisting #endianness, as well as the new #isLittleEndian and #isBigEndian?

Here I am not sure of agreeing.

Going through Smalltalk is evil.

Maybe asking the platform is better, to hide the "EndianDetector".


+1.  EndianDetector is a monstrosity.

Ok, EndianDetector is maybe an ugly name.

But it has a reason to be. Before, to test endianess the system used a Bitmap. And we wanted a standalone class that we could use whether Bitmap is there or not (by becoming an instance of a float into a bitmap or so... I don't remember the details).

So, it is a badly name little child only. We can rename it. Suggestions?

 
Cheers,
Henry

_,,,^..^,,,_
best, Eliot

Reply | Threaded
Open this post in threaded view
|

Re: Deprecations on Smalltalk Image in Pharo 5

Eliot Miranda-2
Hi Guille,

On Tue, Jan 3, 2017 at 1:40 PM, Guillermo Polito <[hidden email]> wrote:
On Tue, Jan 3, 2017 at 8:56 PM, Eliot Miranda <[hidden email]> wrote:
On Tue, Jan 3, 2017 at 2:24 AM, Henrik Johansen <[hidden email]> wrote:
[snip]
 
2) Accessor remains the same, but all references in the base image has been rewritten to use EndianDetector directly...
2) Would not Smalltalk platform be a reasonable place to put this responsiblity, letting it use an EndianDetector behind the scenes? Both the preexisting #endianness, as well as the new #isLittleEndian and #isBigEndian?

Here I am not sure of agreeing.

Going through Smalltalk is evil.

Maybe asking the platform is better, to hide the "EndianDetector".


+1.  EndianDetector is a monstrosity.

Ok, EndianDetector is maybe an ugly name.

No, that's not the reason this is a monstrosity.  Extracting one method into a class of its own, a method that answers something trivial, that as Hwnrik points out is logically to do with Smalltalk platform, that's what's a monstrosity.  I could care less what its called.  But it gratuitously broke lots of code that used it in a very ugly way while adding a large overhead for trivial functionality.  It's a monstrosity.


But it has a reason to be. Before, to test endianess the system used a Bitmap. And we wanted a standalone class that we could use whether Bitmap is there or not (by becoming an instance of a float into a bitmap or so... I don't remember the details).

Yeah, well, there's a VM parameter, there are no end of ways to compute it, but the point is, it can be a single method on Smalltalk platform not a whole class.

_,,,^..^,,,_
best, Eliot
Reply | Threaded
Open this post in threaded view
|

Re: Deprecations on Smalltalk Image in Pharo 5

Henrik Sperre Johansen
In reply to this post by Guillermo Polito

On 3 Jan 2017, at 22:40 , Guillermo Polito <[hidden email]> wrote:



On Tue, Jan 3, 2017 at 8:56 PM, Eliot Miranda <[hidden email]> wrote:


On Tue, Jan 3, 2017 at 2:24 AM, Henrik Johansen <[hidden email]> 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:




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)

 

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?

Cheers,
Henry