SeasideSt / Parasol

Testing web apps in Smalltalk using Selenium WebDriver.
MIT License
31 stars 16 forks source link

port to 3.6.0 #41

Closed dalehenrich closed 4 years ago

dalehenrich commented 4 years ago

It seems that at a minimum, the method CharacterCollection>>substrings: seems to be defined in 3 packages: Grease-GemStone-Core and Parasol-GemStone:

==================== Parasol-GemStone (-) --> image (+)  ====================
Item was removed:
- ----- Method: CharacterCollection>>substrings: (in category '*Parasol-GemStone') -----substrings: separators
-   ^ self subStrings: separators!

So not needed in Parasol ... may not be strictly related to 3.6.0, but I'm not finished yet:)

jbrichau commented 4 years ago

@dalehenrich We allow Parasol to be used without Seaside, so this is why we frequently have dirty packages when several projects include the same method extensions. I agree a common package like Grease, Squeak-Compatibility and Pharo-Compatibility is desirable but it's often impractical to depend on as well.

Is there a specific issue with these extensions or dirty packages that we should care about? Asking to understand.

dalehenrich commented 4 years ago

I list the specific method in the original bug report and the two packages involved ... given that you want to keep Parasol independent of Seaside, I'm inclined to add the method to GLASS1 ...

The reason that I am concerned about dueling packages is that I expect to be writing out packages with changes needed for 3.6.0 and these duplicate methods just make it more complicated to keep things straight.

I have also observed that it isn't possible to reload Seaside once Parasol is in the image (some sort of load loop that I haven't figured out between Parasol and Seaside, but I see that Parasol requires Seaside and Seaside requires Parasol, so that is likely the source of this issue) ... again ... not a problem if you just load the projects, but a real pain when you are doing development --- I am doing active development in 3.5.4 and 3.6.0, so it is a pain to have to rebuild the image, to share changes ...

It's good to know that Parasol is supposed to be loaded independently and I suppose if you are happy with the state of things, I can just quietly step away from the problem --- I have quite a bit on my plate as it is, but I also figure that the best time to fix a problem is when it is right in front of you ...

I'm pretty sure that I can address the CharacterCollection>>substrings: by simply moving the method into Grease (presumably both packages use Grease?) or into GLASS itself and then remove the method from the GemStone packages in both Parasol and Seaside ... So, let me know if you want me to look into the load loop or not , but for now I assume you want me to ignore it:)

dalehenrich commented 4 years ago

Let's make that three packages now:

==================== Grease-GemStone-Core (-) --> image (+)  ====================
Item was removed:
- ----- Method: CharacterCollection>>substrings: (in category '*grease-gemstone-core') -----substrings: aCharacter
-   "Pharo 6+ compatibility"
- 
-   ^ self subStrings: aCharacter!
dalehenrich commented 4 years ago

Grease is part of the base GLASS project these days, so the method is simply not needed in either Seaside or Parasol and no need to add a dependency on Grease (for GemStone) ...

Also, I must have been mistaken about Seaside-Squeak-Compatibility involvement in the dueling methods, because Seaside-Squeak-Compatibility implements String >>substrings: which is redundant but at least not competing ...