Hi Lukas, On 16 Mar 2010, at 21:50, 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 isValidifTrue: [ package addMethod: each compiledMethod isExtension: false ] ].info extensionMethods do: [ :each | each isValidifTrue: [ 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?
Stef said that this was a reminiscent from before deciding he wants to declare the class explicitly, but we agreed that specifying the extension explicitly is not necessary.
Having two dictionaries for methods makes it extremely difficult to move stuff around because always 4 separate cases need to be handled.
I also do not like this part either.
- The fact that compiled methods are stored in the model is very dangerous. 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). 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 model does not store compiled methods, or classes. It only stores symbols.
- 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.
I agree, but this was just a quick thing to see what kind of navigation methods are needed to get the classes and methods. Categories are still to be added.
- The browser displays extension methods on both instance and class-side. When browsing an extended class, the extensions are not displayed.
Yes, these were bugs :). I tried to fix these, but I think I bumped into other problems, so I will stop for now.
Cheers, Doru
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
-- www.tudorgirba.com "Sometimes the best solution is not the best solution." _______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
