- Issue created by @nicxvan
- π©π°Denmark ressa Copenhagen
Moving some description here, from duplicate issue.
- π¨π¦Canada joelpittet Vancouver
In CheckCircularReferencesPass.php line 81: Circular reference detected for service "migrate_boost.module_handler", path: "migrate_boost.module_handler -> config.factory -> config.typed -> migrate_boost.module_handler".
Current code produces this error. I tried uninstalling and pre patch, and installing with patch with the same result.
- π¨π¦Canada joelpittet Vancouver
Same in the title, drupal 10 and drush 13
- π¨π¦Canada joelpittet Vancouver
Sorry, I applied it against 1.x, disregard my comment
- π¨π¦Canada joelpittet Vancouver
The MR still has this alter() static method call which doesn't exist any more:
function migrate_boost_module_implements_alter(&$implementations, $hook) { MigrateBoost::alter($implementations, $hook); }
What's the plan for this?
- π¨π¦Canada joelpittet Vancouver
MigrateBoost::isBoostActive()
seems to be always not set. And the drush command doesn't seem to be available.
I think the command is missing a services yaml like:migrate_boost.commands: class: Drupal\migrate_boost\Commands\MigrateBoostCommands tags: - { name: drush.command }
and I think the namespace for the command should be
namespace Drupal\migrate_boost\Commands;
instead ofnamespace Drupal\migrate_boost\Drush\Commands;
Thoughts?
- π¨π¦Canada joelpittet Vancouver
RE #11, this is the error, I didn't make a commit because I don't have an idea what is supposed to happen with that, I suspect remove
migrate_boost_module_implements_alter
?Error: Call to undefined method Drupal\migrate_boost\MigrateBoost::alter() in /var/www/html/public/modules/contrib/migrate_boost/migrate_boost.module on line 14 #0 /var/www/html/public/core/lib/Drupal/Core/Extension/ModuleHandler.php(552): migrate_boost_module_implements_alter()
- π¨π¦Canada joelpittet Vancouver
Re #12, based on the Issue Summary goal of βletβs also remove the dependency on Drush,β Iβm assuming we can remove the command entirely.
Since we are decorating the module handler, we also donβt need
migrate_boost_module_implements_alter()
.Iβll go ahead and remove them as Iβm just trying to get this working. Let me know if Iβve overstepped in my haste!
- πΊπΈUnited States nicxvan
No worries! HMIA does need to be removed, I don't recall if we need Drush still, I think it hooks into only affect migrations run with Drush.
Happy to have someone reviewing, I only work on this for a personal project so I've not had time to dig in further.
- π¨π¦Canada joelpittet Vancouver
Thanks @nicxvan, I am not sure what HMIA is, I am guessing it's related to inheritance or something but google/ChatGPT didn't turn up good results.
- π¨π¦Canada joelpittet Vancouver
Ah a Drupal acronym I've never heard of, thanks!
- π¨π¦Canada joelpittet Vancouver
Ok this is working for me now. My last commit got rid of
isHookDisabled()
calls without the new $module arg because it wasn't available without implementinggetImplementationInfo()
andbuildImplementationInfo()
andverifyImplementations()
which isn't the end of the world but getting pretty complicated and likely not needed.The
alter
methods are tricky, because they call some of the decorated methods inside themselves. Itβs almost better to just leave them alone. - πΊπΈUnited States nicxvan
I think I can get behind dropping the ability to kill alter, but I think we need to handle invokeAllWith still
- πΊπΈUnited States nicxvan
What do you think of that?
Thanks for all of your help!
- π¨π¦Canada joelpittet Vancouver
Thanks for the review!
Iβll look at it tomorrow when I am in.
- π¨π¦Canada joelpittet Vancouver
@nicxvan I had a look, trying it out IRL right now. The code looks good to me, going to mark this RTBC from my end as it does what is on the Tin ;)
- π©πͺGermany Grevil
Seems to work great! Currently, in the midst of migrating a large Drupal 6 site. If anything goes wrong related to this module, I'll comment here again!
Note, that the introduction of the new Decorator leads to an error when migrating d6 nodes. But that is unrelated to this module but rather a drupal core issue. I just created an issue + fix here: π "core/modules/node/src/Plugin/migrate/source/d6/Node.php" "__construct" method uses variable of type "ModuleHandler" instead of "ModuleHandlerInterface" Active .
Otherwise, RTBC +1!