Problem/Motivation

One of the changes in #3214186: Scan for snippets of process plugins is

--- a/src/Plugin/MigrationPluginManager.php
+++ b/src/Plugin/MigrationPluginManager.php
@@ -23,7 +23,7 @@ class MigrationPluginManager extends DefaultMigrationPluginManager {
       }, $this->moduleHandler->getModuleDirectories());
 
       // Retrieves **/*.yml files but gets rid of /migrations/state/*.yml files.
-      $yaml_discovery = new YamlRecursiveDirectoryDiscovery($directories, 'migrate', 'id', '#/migrations/state/#');
+      $yaml_discovery = new YamlRecursiveDirectoryDiscovery($directories, 'migrate', 'id', '#/migrations/(process|state)/#');

That does not scale well. I think we need an easy way for a developer to set exclude rules while working on a custom migration.

It might be useful to allow include rules as well as exclude rules. For example, a developer working on several related sites might want a single module (perhaps including some custom plugins) and migrations in migrations/site1/, migrations/site2/, ...

Proposed resolution

Either add an alter hook or trigger an event so that other modules can modify the paths to be scanned.

I think we should optimize for simplicity, not flexibility. Instead of giving other modules the list of directories and letting them manipulate it, just let them add include/exclude patterns.

Remaining tasks

User interface changes

API changes

This feature will add to the public API of this module.

Data model changes

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

benjifisher created an issue. See original summary.

matroskeen’s picture

Sounds interesting, let me try :)

matroskeen’s picture

Status: Active » Needs review

I think it would be a nice feature and it perfectly fits within the module scope.
I prefer hooks for such alterations, I find them easier to use.

The merge request is ready for review.

benjifisher’s picture

Status: Needs review » Needs work
matroskeen’s picture

Status: Needs work » Needs review

@benjifisher, thanks for the review!
Can you have another look?

benjifisher’s picture

Status: Needs review » Needs work

I am sorry it has been more than a week since I last looked at this. I made some further suggestions on the MR.

matroskeen’s picture

Status: Needs work » Needs review

Applied requested changes. I think we're getting close :)

benjifisher’s picture

Status: Needs review » Needs work

@matroskeen:

Thanks, I think the new version of the alter hook is a big improvement. I made a few more suggestions on the MR. Back to NW.

matroskeen’s picture

Status: Needs work » Needs review

@benjifisher, thanks for the extensive review!
We can keep going, this is ready for review again ;)

benjifisher’s picture

Status: Needs review » Reviewed & tested by the community

OK, the review looks good.

For a realistic test, I added a custom module and migrations/process/clean_whitespace.yml copied from #3214186: Scan for snippets of process plugins. Then I got an error message:

drush cc plugin
drush ms
In YamlDirectoryDiscovery.php line 114:
                                                                                                                            
  The /app/modules/custom/migrate_test/migrations/process/clean_whitespace.yml contains no data in the identifier key 'id'

Then I added

function migrate_test_migrate_scanner_patterns_alter(&$patterns) {
  $patterns['exclude'][] = '#/migrations/process/#';
}

and cleared all caches, and drush ms does not produce any errors.

Test passed! RTBC

  • Matroskeen committed 7a4e5bb on 1.0.x
    Issue #3214464 by Matroskeen: Let other modules modify the scanning...
matroskeen’s picture

Status: Reviewed & tested by the community » Fixed

Tada! 🥳 Merged into 1.0.x.
@benjifisher, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

sivakarthik229’s picture

Hi All,

I am getting similar error mentioned in #11 even after adding the migrate_scanner hook.

Please help me to fix the error.