Problem/Motivation
MigrationPluginManagerInterface has the following added to its interface:
createInstances
createStubMigration
But it is missing createInstancesByTag. I suggest this is an oversight and should be added.
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | 2912803-6.patch | 1.56 KB | jofitz |
| #2 | 2912803-2.patch | 1.56 KB | jofitz |
Comments
Comment #2
jofitzComment #3
maxocub commentedI think this needs a change record.
Comment #4
heddnCR added.
Comment #6
jofitzPatch in #2 no longer applies. Re-rolled.
Comment #7
quietone commentedTest passing so back to RTBC
Comment #8
gábor hojtsyHm, this could potentially be a can of worms AKA it definitely looks like a backwards incompatible change. If contribs / custom modules don't implement this they would whitescreen right after updating. Any other options?
Comment #9
heddnI agree wwith the sentiment in #8. However, if I stop and thing about how and why someone would alter out the plugin manager... then I realize that no one has done that. It is a pretty difficult activity for no real benefit. I doubt anyone has done that. Isn't there some docs about implementations of interfaces by a single class and that essentially being internal?
Comment #10
phenaproximaIt seems quite unlikely that anyone will have implemented their own MigrationPluginManagerInterface from scratch. The only example I can think of in core is over in #2908282: Throw exception for source plugins without a source_module property, in which Migrate Drupal does replace the plugin.manager.migration service -- but it does it by extending the base class.
I think, in practice, this is a BC-safe change, so I'm marking it RTBC. But if the framework/product managers disagree, then we'll see where we can go from here...
Comment #11
larowlanI'm not sure on this either.
The alternative is to add a new interface and have MigrationPluginManager implement it.
We can then remove it and merge them in D9.
I've asked the other framework managers and the release manager.
Comment #13
heddnComment #15
heddnRandom failure
Comment #16
xjmYes, our API policy allows us to add methods to interfaces that have a corresponding base class that would be used in most cases:
https://www.drupal.org/core/d8-bc-policy#interfaces
So this is OK with a change record (which it has).
Thanks!
Comment #17
heddnComment #18
heddnComment #19
catchCommitted d778f80 and pushed to 8.5.x. Thanks!
Comment #22
quietone commentedPublished change record.