Allow Provider Sources to choose a custom superclass #275
Conversation
alassek
force-pushed
the
provider-class-override
branch
from
4c43fd5
to
2cad076
Compare
There was a problem hiding this comment.
This is really great, @alassek! Thank you so much for making it happen 😄
I've left a couple of notes for you that should hopefully be enough for you to get this into a ready-to-merge state.
Before we merge, though, would you be up for making a matching change to hanami that uses this? I think that this feature should allow us to get rid of our custom provider registrar in Hanami? That would be a nice outcome if possible — just one piece of customisation instead of two.
spec/integration/container/providers/custom_provider_superclass_spec.rb
Outdated
Show resolved
Hide resolved
Certainly, I wanted to pause to get feedback on the approach before continuing, but that is the next step and shouldn't require much effort. |
alassek
force-pushed
the
provider-class-override
branch
from
2cad076
to
565633c
Compare
alassek
force-pushed
the
provider-class-override
branch
3 times, most recently
from
d6b30e6
to
583cec6
Compare
There was a problem hiding this comment.
Thanks for refreshing this PR, @alassek!
It looks like you've found a nice arrangement here, with none of the provider build_source proc awkwardness that I introduced in in my spike on this at hanami/hanami#1417 (comment).
So this looks good to me overall! I've just left 2 comments for you. One is superficial, but the other one (regarding provider_source_options) is a more important aspect of the functionality. Let me know what you think about that, and after that is sorted, we can merge this in! 😄
alassek
force-pushed
the
provider-class-override
branch
6 times, most recently
from
75c1073
to
cb90a32
Compare
Relates to hanami/hanami#1417 The motivation for this is to allow consuming frameworks of dry-system to define their own superclass for providers, in order to add their own method apis to the class. The is only used for user-defined providers with a block implementation. External providers in a group do not allow this, because their superclass is defined ahead of time when they are added to the source registry. If an external provider source wants to use a different superclass, they can define a concrete class of their own instead. The custom superclass is assumed to be a child of Dry::System::Provider::Source. In addition to `provider_source_class`, ProviderRegistrar contains `provider_source_options` as an extension point for subclasses to send custom intialization params to the source class.
alassek
force-pushed
the
provider-class-override
branch
from
cb90a32
to
a8caa31
Compare
|
@timriley Okay this should be caught up to all your latest feedback! |
|
@alassek FYI, I just pushed a commit here, which made the Hanami specs go green again on my machine. Things were breaking in the tests that did |
timriley
force-pushed
the
provider-class-override
branch
from
ae1ff81
to
1cc03d9
Compare
|
@alassek Are you happy with this PR (and hanami/hanami#1446) in their current states? |
|
@timriley Yes, thank you for figuring out the I'm happy with this. |
|
Thanks for all your efforts getting this across the line, @alassek!! |
Relates to hanami/hanami#1417
The motivation for this is to allow consuming frameworks of dry-system to define their own superclass for providers, in order to add their own method apis to the class.
This is only used for user-defined providers with a block implementation.
External providers in a group do not allow this, because their superclass is defined ahead of time when they are added to the source registry. If an external provider source wants to use a different superclass, they can define a concrete class of their own instead.
The custom superclass is assumed to be a child of
Dry::System::Provider::Source.