On Mar 16, 2010, at 9:50 PM, Lukas Renggli wrote:
I have some comments, to both the package implementation and the Glamour browser. The other browser didn't work, I guess I miss some extra package.
- I used the code below to import the existing packages into the new model. Maybe something like this should be included on the class side of RPackageOrganizer to have a realistic model?
| package | PackageOrganizer default packages do: [ :info | package := RPackage2 named: info packageName. info classes do: [ :each | package addClassDefinition: each ]. info coreMethods do: [ :each | each isValid ifTrue: [ package addMethod: each compiledMethod isExtension: false ] ]. info extensionMethods do: [ :each | each isValid ifTrue: [ package addMethod: each compiledMethod isExtension: true ] ] ] displayingProgress: 'Importing'
Then I opened the glamour browser using:
GTCoder openOn: RPackageOrganizer default
- I find it quite strange that I have to declare if a method is an extension or not. Isn't a that obvious when looking at the defined classes?
Historically it was like you said in implementation 1 and 2, then I removed the defined classes (implementation 3), then I realized oops shit, we can have package with class but no methods (and in this case we should always when removing a method check if this is the last one and if there is none in the metaclass to trigger a classremoved event), so I reintroduced defined classes as a separate data (implementation 4 = now). Now I think that we should remove isExtension to go back to the first implementation. I guess that this is a todo
Having two dictionaries for methods makes it extremely difficult to move stuff around because always 4 separate cases need to be handled.
I discussed that point with doru when you went off your office.
- The fact that compiled methods are stored in the model is very dangerous.
There should not be or doru changes my code!!!
You might hold onto compiled methods that have long been replaced with other ones. Just by playing a bit with the model I run into that situation (I don't know how). I think just keeping the selector internally would be much safer and solve all kinds of troubles (exactly like this is done for the classes).
but this is like that. Doru?
You'll have to check anyway if the method is still present when you iterate over the methods. A single use of #doSilently: (and there are lots of them in the image) can completely screw up the complete package model.
- The browser displays nicely the extended classes, but for the methods I don't see the protocols and the complete set of selectors implemented. I think these things should be part of the browser, otherwise we don't see if the package model can answer these queries quick enough.
- The browser displays extension methods on both instance and class-side. When browsing an extended class, the extensions are not displayed.
That's it for the moment. I see a cool model emerging :-)
Lukas
-- Lukas Renggli http://www.lukas-renggli.ch
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project