-
Notifications
You must be signed in to change notification settings - Fork 85
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update tonel dependency to Pharo12 branch #1735
Conversation
During the development of Pharo 12 we will bring some changes to tonel such as an improved way to deal with packages and tags and maybe a way to set the deprecated aliases of a class. This change make Iceberg depend on the branch Pharo12 of the tonel repository. This branch has one change ahead of the master branch that is the introduction of TonelWriterV3. This new writer will export the package and the tag of classes on top of the category. It also update the reader to be able to read this new format and try to be as backward compatible as possible.
@jecisc / @tesonep / @guillep / @MarcusDenker / @estebanlm There is a unified property API for classes and methods in Pharo already to add properties. If I'm not mistaken this was introduced by @MarcusDenker: Object propertyAt: #alias put: 'OldName'.
(Object propertyAt: #alias) inspect I remember an old discussion with @estebanlm about Tonel and I think the properties of classes and methods are not serialized in current versions of Tonel. If there is work now being done on a new TonelWriterV3 maybe this is the way forward to generizalize and serialize the properties (which then could be used for all kinds of different meta annotations). |
Like with methods, where we have properties (not serialized with code) and pragmas (part of code), this should not store all properties. Instead we should have something similar to a progma, which is rendered as part of the fluid class definition, too |
This first change does not deal with the properties. But the new class deprecation system does. For now it works in initialize methods on the class side. But we want to add it to the FluidBuilder and the TonelV3 format. It can be an idea indeed to think of a way to extend the class builder and the Tonel format to export those kind of properties. We need to think about it (and right now my priority is to remove the concept of categories as package-tag before the end of my contract :p). But as Marcus said, it cannot be as easy as exporting the properties since some of them are used for system internals and should not be persisted. |
Maybe we should think about transient vs. persistent properties. Another thing I noticed in this regard: some slots typically generate the accessor methods dynamically/automagically. Tonel serialization catches them nonetheless and the generated methods are therefore always marked dirthy in Iceberg although autogenerated/recreated - which is strange and not nice. So there is no way yet in Pharo to mark methods as "transient" - so they do not need to be stored in git by the Tonel writer. |
@estebanlm This PR now passes, should we integrate? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@astares yes indeed. We plan other changes to tonel in Pharo 12 and we want to create a new release with the TonelV3 format of the Pharo 12 release at that time. But we still want to integrate the current change in Pharo 12. In time of the release, the tag will be created |
During the development of Pharo 12 we will bring some changes to tonel such as an improved way to deal with packages and tags and maybe a way to set the deprecated aliases of a class.
This change make Iceberg depend on the branch Pharo12 of the tonel repository.
This branch has one change ahead of the master branch that is the introduction of TonelWriterV3. This new writer will export the package and the tag of classes on top of the category. It also update the reader to be able to read this new format and try to be as backward compatible as possible.