search

GemTalk / Sparkle

MIT License
11 stars 5 forks source link

Move Announcements into separate repository #51

Closed kurtkilpela closed 3 years ago

kurtkilpela commented 3 years ago

Announcements is supported by GemStone, Dolphin, and Pharo. In order for RSR to take advantage of it, it will need to be migrated out of the Sparkle repository as RSR shouldn't depend upon Sparkle.

martinmcclure commented 3 years ago

Sparkle has never been considered Announcements' permanent home. Feel free to move those packages out of the Sparkle repo and into RSR or into its own repository.

dalehenrich commented 3 years ago

Separate repository probably makes sense ... @kurtkilpela let me know if you want me to do the heavy lifting to move Announcements to it's own Rowan project and add Announcements dependencies to both Sparkle and RSR...

kurtkilpela commented 3 years ago

@dalehenrich I created another repo w/ Announcements. GemTalk/Announcements has rowan components but I'm not sure they are correct. Getting those verified and handling the Sparkle side would be good.

dalehenrich commented 3 years ago

@kurtkilpela it looks like the tests attribute is not being used ... the tests package is conditional on gemstone ... if the intention of the tests attribute was to make the tests conditionally loadable, then I can fix that ... if you always wants the tests to be loaded (as it is currently defined) then I would move the test package into the gemstone/Announcements package ...

depends upon what you want ...

kurtkilpela commented 3 years ago

I copied the files from Sparkle and edited them. It makes sense to me that the tests are conditionally loadable instead of always loaded. Thanks @dalehenrich.

dalehenrich commented 3 years ago

Okay ... I'll do that (tomorrow) ... then I'll make the necessary changes to both Sparkle and RSR and give them a test load or two ...

dalehenrich commented 3 years ago

@kurtkilpela and @martinmcclure , I have tested the bootstrap loading of Sparkle/RemoteServiceReplication/Announcements and RemoteServiceReplication/Announcements... doing a bootstrap load of Sparkle or RemoteServiceReplication(into a fresh rowan extent) works fine (including the case where the Announcement project has not been cloned yet).

If you attempt to reload RemoteServiceReplication/Announcements into an extent where RemoteServiceReplication is already loaded, the load works fine..

However if one attempts to reload Sparkle/RemoteServiceReplication/Announcements into an extent where Sparkle and RemoteServiceReplication are already loaded then you hit GemTalk/Rowan#680.

I will get busy (tomorrow) with fixing GemTalk/Rowan#680, but we won't be able to merge my dkh_issue_51 branches (for both Sparkle and RemoteServiceReplication) and expect folks to be able to just update their images until the bug has been fixed and they've updated to the newer version of Rowan:master2.1.

@kurtkilpela, since RSR isn't affected you could start using the Announcements project on a topic branch by merging my dkh_issue_51 branch into your branch without having to wait for my bugfix ...

dalehenrich commented 3 years ago

Now that I've reproduced the bug in a Rowan test, I realize that it isn't likely that any of the Sparkle/RSR developers even have a Rowan clone attached to their stone, so it is a moot point that Sparkle and RSR cannot be updated without a bootstrap.

The code is ready and I'll submit PRs at my leisure (probably after I've fixed the bug), ping me if you want an earlier PR ...