OrleansContrib / OrleansTestKit

Unit Test Toolkit for Microsoft Orleans
http://dotnet.github.io/orleans
MIT License
78 stars 43 forks source link

Removed `sealed` keyword #92

Closed jkonecki-techfabric closed 4 years ago

jkonecki-techfabric commented 4 years ago

Several key classes (StorageManager, TestStorage, TestKitSilo) are marked as sealed.

This means their behaviour cannot be customized.

I'm currently using a storage provider and grains that are event-sourced, and need custom behaviour around state handling (state is not just a single object, it's a stream of events). I also want to allow for richer tests when reading / writing state.

The alternative is to fork the whole project, wchich I would like to avoid.

Would you please consider dropping sealed keywords? I'm happy to make a PR.

It's a bit urgent for me, so would hugely appreciate your response.

seniorquico commented 4 years ago

@jkonecki-techfabric Sounds fine to me. But is sealed enough? Are there methods that would also need to be marked virtual? If you can, take a pass over the classes and submit a PR.

I believe the last release process ran fully automated. Publishing a new version on NuGet should be as simple as tagging the desired commit after the PR is merged.

jkonecki-techfabric commented 4 years ago

Yes, TestStorage methods would be virtual. I'll make a PR shortly.

Thank you very much for a prompt response!


From: Kyle Dodson notifications@github.com Sent: Friday, July 3, 2020 6:08:50 PM To: OrleansContrib/OrleansTestKit OrleansTestKit@noreply.github.com Cc: Jakub Konecki jakub@techfabric.co; Mention mention@noreply.github.com Subject: Re: [OrleansContrib/OrleansTestKit] Removed sealed keyword (#92)

@jkonecki-techfabrichttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_jkonecki-2Dtechfabric&d=DwMCaQ&c=euGZstcaTDllvimEN8b7jXrwqOf-v5A_CdpgnVfiiMM&r=J5xwzqk2JV6JmMQ8LmNyE6cRZCpM-ICHZBNHSFKl9k8&m=EMcUmuXWBJmyZdcLlLQNd_Yme0kDMb3AZCEvylC-0-c&s=xpXlwhsRo09ZfuU3JbQnZhuVmtANlhHvx1v69pfbJdg&e= Sounds fine to me. But is sealed enough? Are there methods that would also need to be marked virtual? If you can, take a pass over the classes and submit a PR.

I believe the last release process ran fully automated. Publishing a new version on NuGet should be as simple as tagging the desired commit after the PR is merged.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_OrleansContrib_OrleansTestKit_issues_92-23issuecomment-2D653625859&d=DwMCaQ&c=euGZstcaTDllvimEN8b7jXrwqOf-v5A_CdpgnVfiiMM&r=J5xwzqk2JV6JmMQ8LmNyE6cRZCpM-ICHZBNHSFKl9k8&m=EMcUmuXWBJmyZdcLlLQNd_Yme0kDMb3AZCEvylC-0-c&s=NvBzGlFYxPPQ7YQgQ9fCn2kMcqiSEUuasb0BHvbXEvU&e=, or unsubscribehttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_ANYCISNCPIRI3KECWOAVHV3RZYGCFANCNFSM4OP6SALA&d=DwMCaQ&c=euGZstcaTDllvimEN8b7jXrwqOf-v5A_CdpgnVfiiMM&r=J5xwzqk2JV6JmMQ8LmNyE6cRZCpM-ICHZBNHSFKl9k8&m=EMcUmuXWBJmyZdcLlLQNd_Yme0kDMb3AZCEvylC-0-c&s=E9zht-8rvI4soL2MtgizhzQftzkAwibLXSCGXDbENuc&e=.