Remove CMI dependency from Migration Templates

This is much simpler. I like simple. I want to get @alexpott's opinion on this, since he's a CMI guru and can offer insight on what, if anything, we might be losing by this change.

+++ b/core/modules/migrate/src/MigrateTemplateStorage.php
-  const MIGRATION_TEMPLATE_DIRECTORY = 'migration_templates';

Let's keep the ability to override this in the constructor. We don't need the constant, but it should be a default parameter on __construct().

+    $yaml_parser = new Yaml();

This is a micro-optimization, but I believe Yaml::decode() can be called statically. Can you do that instead, rather than going through the overhead of instantiating a parser?

+          $templates[$file->getBasename('.yml')] = $yaml_parser->decode(file_get_contents($file));

The template might include an id. Let's check for that and use it as the key, if it's defined. Otherwise, use the file's base name (sans .yml extension, of course).

Looks OK to me, but in addition to @alexpott I'd also like to get @mikeryan's input before RTBCing.

Yes! I should have thought of this. Nice work @neclimdul

+++ b/core/modules/migrate/src/MigrateTemplateStorage.php
@@ -57,18 +69,15 @@ public function findTemplatesByTag($tag) {
+    foreach ($this->moduleHandler->getModuleDirectories() as $directory) {
+      if (file_exists($directory . '/' . $this->directory)) {
+        $files = new \GlobIterator($directory . '/' . $this->directory . '/*.yml');
+        foreach ($files as $file) {
+          $templates[$file->getBasename('.yml')] = Yaml::decode(file_get_contents($file));
+        }
+      }
     }

Let's write stuff compatible with #2304461: KernelTestBaseTNG™. See the patch for how it removes usage of GlobIterator.

It's simple. It passes the tests. @alexpott confirmed on IRC that there was no particular need (beyond convenience) for MigrateTemplateStorage to use CMI's discovery mechanisms, and @mikeryan and I are usually on the same page regarding architecture/simplicity.

All this adds up to an RTBC from me. Thanks, @neclimdul!

I agree lets do this

I like this.

Committed cc8e7c3 and pushed to 8.0.x. Thanks!

Since you are destroying all my migrate work anyways with this template business, why don't you finish and remove them completely from the config entity system as well? Reopen #2462233: Migrations should not use the configuration entity system and be done

Edit: turns out we didn't remove config entity support but amended it; however the change record said

If your module supplies migrations, they should go in the migration_templates directory, without the migrate.migration prefix, so that the migration tools can selectively install them if and only if they're needed.

which is wrong. The CR will be fixed.

Also how did this got in without benjy seeing it?

Edit: I am told it's a minor change and as per https://www.drupal.org/governance/core only significant changes need subsystem maintainer approval. Well, then we will (will??) have a dictionary war over the word "significant" :)

@chx coupling how migrate discovers its templates to CMI default configuration discovery makes no sense. @benjy rtbc-d the template system #2463909-58: Migrations should support non-installed default configurations (templates) and this is a minor change. In fact it is really just a followup to that issue. It is not okay to say

Since you are destroying all my migrate work anyways with this template business, why don't you finish and remove them completely from the config entity system as well?

And whilst I appreciate the courage of just crossing out and not removing it, I feel they we should acknowledge that @mikeryan, @phenaproxima, @benjy and others are all working extremely hard to deliver an excellent migration system for Drupal 8.