Remove useless config action.settings.recursion_limit

+++ /dev/null
@@ -1,9 +0,0 @@
diff --git a/core/modules/action/migrations/action_settings.yml b/core/modules/action/migrations/action_settings.yml
deleted file mode 100644
index 4d332f801d..0000000000
--- a/core/modules/action/migrations/action_settings.yml
+++ /dev/null
@@ -1,16 +0,0 @@

We can't just delete migrations. They are technically plugin definitions, so they need to be deprecated.
If the actions_max_stack variable truly has no counterpart in D8 and the migration is useless, then you can to set it to use a null source and destination, remove it from the Drupal 6 and Drupal 7 groups so it isn't run, mark it deprecated and throw a trigger error somewhere, probably the migration constructor.

Migrate checks all migrations for source plugins existing. It happens in \Drupal\migrate\Plugin\MigrationPluginManager::getDiscovery . Seems like \Drupal\Tests\action\Kernel\Migrate\d6\MigrateActionConfigsTest can not be passed with such changes.

I'm going to need to dig into this a bit more and see what they best thing to do it. We've never deprecated a whole migration without a replacement before that I'm aware of, so we need a way to render it inert while still allowing it to be instantiated and ran. I thought we could do it with th enull plugins, but since they aren't stub migrations, the requirements system is rejecting them. We may have to do something hacky like using an embedded data source with no rows (which might not even work)

Since this is deprecating a migration adding some migrate tags.

The migration handling here looks to be correct for now, and has my sign-off as a migration sub-system maintainer. Migrations are plugins, they cannot simply be removed or it is considered a BC break because someone might be relying on their plugin id. The plugin implementation can change without a bc break provided the id and signature remain. This migration serves no purpose as the config it imports is never used. Removing its functionality is not a BC break because the plugin implementation still exists with all methods.

This patch renders the migration useless without removing it. A normal plugin would be deprecated and marked for removal, but we have recently realised that we do not currently have a mechanism to deprecate a migration yml and mark it for removal. A follow up to this will be in #3039240: Create a way to declare a plugin as deprecated. When we decide on and implement a process for deprecation of migration ymls, we will include this migration as the first one to be deprecated.

The remainder of the patch and test changes look correct to me, but I will leave for someone with more domain knowledge around this particular config to RTBC.

Looks pretty neat. We need the Update test to verify that the configuration deleted after the update hook.

+++ b/core/modules/config/tests/config_import_test/src/EventSubscriber.php
@@ -87,10 +87,10 @@ public function onConfigImporterMissingContentTwo(MissingContentEvent $event) {
+    if ($config->getName() == 'automated_cron.settings') {

How this is related to the scope of this issue? Shouldn't this be a separate issue?

Solving testing problems.

This config was made obsolete by #1846172: Replace the actions API

Then, another thing the current action system does is tracking the number of recursive calls and caring about recursion. I don't think that's something the action system should take care of. It should provide me with an action that I can execute - whether this creates a recursive loop is up to the calling code; usually (think VBO) it won't. Recursions may occur in the regular trigger/rules use-case, but then it's caused by the triggering logic so the check should be there.

I'm not sure we've removed a whole config file before. I wonder about the BC implications of this and whether a module's configuration forms part of its API. Other modules could be doing something like \Drupal::config('action.settings')->get('recursion_limit') for what its worth. https://www.drupal.org/core/d8-bc-policy does not mention config.

Discussed with @catch. This issue needs a change record - https://www.drupal.org/node/add/changenotice?field_project=3060&field_is...

@catch felt that config is along the lines of schema or data structure. Meaning he think's we can remove it with a change record. There's bound to be cases where that's harder because people actually check the value directly, but as a general rule.

The tests run to me!

+++ b/core/modules/action/action.install
@@ -0,0 +1,13 @@
+
+/**
+ * Removes the action.settings configuration.
+ */
+function action_update_8001() {
+  \Drupal::configFactory()->getEditable('action.settings')->delete();
+}

This should be a hook_post_update_NAME() since it's just removing configuration not changing schema at all.

One more thing - we should probably have a test to ensure the hook_post_update_NAME() works, there is very similar test coverage in #2893795: Remove serialization.module BC layers for one example.

Assuming this comes back green it looks ready to go for me otherwise.

Updated the change record a little bit. Patch looks good to me, so RTBC.

Committed 54e0602 and pushed to 9.0.x. Backported to 8.9.x. Thanks!