[pharo-project/pharo-core] 6f764a: 30830

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

[pharo-project/pharo-core] 6f764a: 30830

Eliot Miranda-3
  Branch: refs/heads/3.0
  Home:   https://github.com/pharo-project/pharo-core
  Commit: 6f764a96b114e9f78ea25bb8f7042e0d6fc47bd0
      https://github.com/pharo-project/pharo-core/commit/6f764a96b114e9f78ea25bb8f7042e0d6fc47bd0
  Author: Jenkins Build Server <[hidden email]>
  Date:   2014-04-24 (Thu, 24 Apr 2014)

  Changed paths:
    A Kernel.package/Class.class/instance/accessing class hierarchy/addSubclass_.st
    M Slot.package/PharoClassInstaller.class/instance/migrating/updateClass_to_.st
    M Slot.package/PharoClassInstaller.class/instance/notifications/basicClassDefinitionChangedFrom_to_using_.st
    M Slot.package/SlotClassBuilder.class/instance/private/applyAndUpdateSharedVariableOrSharedPool_.st
    M SlotTests.package/SlotClassBuilderTest.class/instance/running/tearDown.st
    A SlotTests.package/SlotClassVariableTest.class/README.md
    A SlotTests.package/SlotClassVariableTest.class/definition.st
    A SlotTests.package/SlotClassVariableTest.class/instance/tests/testClassVariableDoesNotDuplicatesSubclassesOfSuperclass.st
    M Traits.package/TClass.class/instance/accessing class hierarchy/addSubclass_.st
    A Traits.package/Trait.class/instance/accessing class hierarchy/addSubclass_.st

  Log Message:
  -----------
  30830

http://files.pharo.org/image/30/30830.zip


Reply | Threaded
Open this post in threaded view
|

Re: [pharo-project/pharo-core] 6f764a: 30830

Sean P. DeNigris
Administrator
GitHub wrote
  Log Message:
  -----------
  30830

http://files.pharo.org/image/30/30830.zip
Which issues did this include?
Cheers,
Sean
Reply | Threaded
Open this post in threaded view
|

Re: [pharo-project/pharo-core] 6f764a: 30830

Marcus Denker-4

On 24 Apr 2014, at 14:21, Sean P. DeNigris <[hidden email]> wrote:

> GitHub wrote
>>  Log Message:
>>  -----------
>>  30830
>>
>> http://files.pharo.org/image/30/30830.zip
>
> Which issues did this include?
>
https://pharo.fogbugz.com/f/cases/13028/

I integrated it with a .cs as it did not work as a slice.

I will rever 830, 831 and 832 (which is just that + reload of dirty + the postscript for the change).

But then we need to fix the issue of 13028…

        Marcus
Reply | Threaded
Open this post in threaded view
|

Re: [pharo-project/pharo-core] 6f764a: 30830

Henrik Sperre Johansen

On 24 Apr 2014, at 2:24 , Marcus Denker <[hidden email]> wrote:

>
> On 24 Apr 2014, at 14:21, Sean P. DeNigris <[hidden email]> wrote:
>
>> GitHub wrote
>>> Log Message:
>>> -----------
>>> 30830
>>>
>>> http://files.pharo.org/image/30/30830.zip
>>
>> Which issues did this include?
>>
> https://pharo.fogbugz.com/f/cases/13028/
>
> I integrated it with a .cs as it did not work as a slice.
>
> I will rever 830, 831 and 832 (which is just that + reload of dirty + the postscript for the change).
>
> But then we need to fix the issue of 13028…
>
> Marcus
Maybe it’s just me, but isn’t it entirely wrong to fix subclasses being kept unmodified when a Shared Variable is added using change propagation?
To me, it seems that mechanism is intended only to be used when a class change actually affects its subclass’ definition (iow, when Fields are modified...).

I mean, it *should* be possible to setup the new class being migrated to with proper subclasses instvar, though I can’t exactly see where that is through all the layers of indirection…

Cheers,
Henry

signature.asc (859 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [pharo-project/pharo-core] 6f764a: 30830

Henrik Sperre Johansen

On 24 Apr 2014, at 3:19 , Henrik Johansen <[hidden email]> wrote:

>
> On 24 Apr 2014, at 2:24 , Marcus Denker <[hidden email]> wrote:
>
>>
>> On 24 Apr 2014, at 14:21, Sean P. DeNigris <[hidden email]> wrote:
>>
>>> GitHub wrote
>>>> Log Message:
>>>> -----------
>>>> 30830
>>>>
>>>> http://files.pharo.org/image/30/30830.zip
>>>
>>> Which issues did this include?
>>>
>> https://pharo.fogbugz.com/f/cases/13028/
>>
>> I integrated it with a .cs as it did not work as a slice.
>>
>> I will rever 830, 831 and 832 (which is just that + reload of dirty + the postscript for the change).
>>
>> But then we need to fix the issue of 13028…
>>
>> Marcus
>
> Maybe it’s just me, but isn’t it entirely wrong to fix subclasses being kept unmodified when a Shared Variable is added using change propagation?
> To me, it seems that mechanism is intended only to be used when a class change actually affects its subclass’ definition (iow, when Fields are modified...).
>
> I mean, it *should* be possible to setup the new class being migrated to with proper subclasses instvar, though I can’t exactly see where that is through all the layers of indirection…
>
> Cheers,
> Henry
What is the purpose of self subclasses: nil in Class >> superclass:methodDictionary:format:?

Is there any case it might be needed except when format changes, and old subclasses are invalid? (and will it ever crash even then until change is propagated and the old instances become’d into subclasses of proper format?)

If not, it might be a decent change to simply change that method to:

Class >> superclass: aClass methodDictionary: mDict format: fmt
        "Basic initialization of the receiver"
        self format = fmt ifFalse:[self subclasses: nil. ].
        super superclass: aClass methodDictionary: mDict format: fmt.

so only cases where format changes are actually responsible for re-registering properly updated subclasses. (which excludes addition of shared vars/pool variables)

Cheers,
Henry

signature.asc (859 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [pharo-project/pharo-core] 6f764a: 30830

stepharo
Hi henryk

thanks for your feedback, we need more people looking at changes.

Stef