ObjectProfile / Roassal2

Agile Visualization Engine for Pharo and VisualWorks
http://AgileVisualization.com
MIT License
26 stars 20 forks source link

Spec2 and Pharo10+ compatibility #65

Closed JanBliznicenko closed 2 years ago

JanBliznicenko commented 2 years ago

The gratest change is creation of Roassal2Spec2 package that allows using Roassal2 with Spec2.

In Pharo 10+, only this Roassal2Spec2 is loaded (Roassal2Spec is not, Spec1 does not exist there so it is not even loadable) In Pharo 9, both original Roassal2Spec and Roassal2Spec2 is loaded (and usable). In Pharo 8-, Spec2 is too outdated so only original Roassal2Spec is loaded.

There are also few minor changes to accomodate differences in Pharo10+, like way of getting available fonts or fixing order of world menu items.

After these changes, Roassal 2 is usable with both Spec1 (in Pharo 8 and 9) and Spec2 (in Pharo 10 and 11). I did not test Pharo 7- as it is already recommended to use older fixed branch.

Current builds of OpenPonk use Spec2, are build on Pharo 10 and use this Roassal2 commit for that.

akevalion commented 2 years ago

I will review it

akevalion commented 2 years ago

The code looks good, but there are some error in the CI server, related to pharo10.

JanBliznicenko commented 2 years ago

Hello, yes, I did not realize that while all stuff I use works, other, that I did not even load, does not. For example anything related to GT/GLM cannot load in Pharo10+ (so the "NoGlamour" group should be what default is).

akevalion commented 2 years ago

When I try to load your branch I got this error

image

Then you will need to modify the dependencies for Glamour, when you load pharo10. let me know if you need help with that

JanBliznicenko commented 2 years ago

@akevalion I managed to modify the baseline so the Glamour stuff is not loaded in Pharo10. Question is whether is it acceptable to not load it at all or if it should be updated in a way that GT stuff would be loadable again. Currently the CI fails because smalltalk.ston specifies GT tests should run and I am not sure how to disable it just for Pharo10.

akevalion commented 2 years ago

I think we are not used anymore smalltalk.ston CI for pharo8 and pharo9 is calling this file https://github.com/ObjectProfile/Roassal2/blob/master/scripts/runTests.st

JanBliznicenko commented 2 years ago

Ok, does it not ignore the acutal code in the pull request / fork / branch / whatever and always just load current master? If that were true (not sure it is), it would work for checking after commit is added to master, but would not work for other branches and PRs, like this one.

akevalion commented 2 years ago

Yes you are right, that script does not consider to load and test the new branch

akevalion commented 2 years ago

For now if you can load your code in pharo10, that would be great. I will fix runTests.st then to run with new branches

JanBliznicenko commented 2 years ago

Yes, to confirm it, I created experimental PR where I kinda removed most of the Roassal2 :) https://github.com/ObjectProfile/Roassal2/pull/66 Should not be loadable and those few tests should all fail, but it is green.

I made this one ready for review. Current state is that this version of baseline does not load anything related to GT at all, all Roassal and Trachel tests ran locally are green and stuff I use (which is, however, just a fraction of all Roassal packages) actually works in Pharo 10 and 11.