Rename i18n migrations to _translation

Renamed the 13 files listed and also came across:

• core/modules/config_translation/src/Plugin/migrate/source/d6/I18nProfileField.php
• core/modules/config_translation/tests/src/Kernel/Plugin/migrate/source/d6/I18nProfileFieldTest.php

Corrected the docs.

There was a missed rename of VariableTranslation/VariableTranslationTest. Hopefully this fixes that up.

Discussed the rename with @catch and @alexpott, they pointed out that altough this is migration templates in stable modules and migrate_drupal and migrate_drupal_ui, it would still be best to provide a BC layer and deprecate the old classes instead of renaming.

@Gábor Hojtsy How to deprecate the .yml files? I found an ongoing related issue on this 'https://www.drupal.org/node/2627938'.

We don't have any standard for deprecating .yml files yet. Since that service deprecation issue, Symfony has added a mechanism to deprecate services, but that doesn't help us here.

I'd suggest:

1. Deprecate the classes (they can inherit from the new ones)
2. Copy the yml files instead of rename, and don't explicitly mark them deprecated.

We'll then need a follow-up to figure out how to mark the YAML as deprecated. If it gets used then the classes will throw deprecation notices, so that bit is fine, so we're really just looking for a way to document in the file itself the deprecation so that people landing on the file will be able to figure it out, and so that they're easy to identify for removal when 9.x opens.

I don't think we need to deprecate the yml files. Renames for them is fine. Remember that these are templates. And only used a single time during setup of migrate. Once this is committed, then the old templates will never be used. However, it is a good point that we need to think about copy/deprecate. I think that means we need to clone i18nVariable => VariableTranslation, I18nProfileField => ProfileFieldTranslation. Then mark i18nVariable & I18nProfileField as deprecated. And clone the tests for these too.

Even still, I've opened #2884407: [Policy, no patch]: Determine method to deprecate yml files for figuring out a way to mark something in yml as deprecated.

Would someone building migrations not expect the templates be there and therefore an ongoing migration process would fail with a core update => BC break?

Building migrations, folks copy/paste from migrate_templates. Or they run drush migrate-upgrade --configure-only (my personal favorite) or the use the migrate_drupal_ui module. For any of these, once the source code is updated, then folks *should* copy/paste from the non-deprecated template. Or the ui or drush would auto pick the non-deprecated template. The old templates should just go away for new migrations.

For existing migrations already built by one of the above methods, as long as the source classes exist, then the existence of the templates is irrelevant.

@heddn: ok, I don't personally have experience building migrations. Would be nice to confirm that from another migrate team member and @catch or otherwise get @catch qualify his suggestion.

. Or they run drush migrate-upgrade --configure-only (my personal favorite) or the use the migrate_drupal_ui module. For any of these, once the source code is updated, then folks *should* copy/paste from the non-deprecated template.

This is a good answer on the templates, so let's remove those.

Copying the classes will mean that existing migrations using them will continue to work (with deprecation messages).

Tidying up a few coding standards issues.

From #16, I think we are missing test coverage of the legacy i18n classes. Those need to be cloned, not renamed. And we are missing an @see to the CR. See https://www.drupal.org/core/deprecation#how for steps to deprecate.

Tagging novice, because all we need to do is copy, not rename the test cases. We'll also need to write a CR, which is probably less novice. Meaning the writing, not the @see. The link to the CR is definitely novice.

I did a quick review of the CR. It looks good. Just a couple small nits and this is good to go. Still leaving the Novice tag, in case someone wants to pick up the nits.

1. +++ b/core/modules/config_translation/src/Plugin/migrate/source/d6/I18nProfileField.php
   @@ -2,7 +2,9 @@
   +@trigger_error('The ' . __NAMESPACE__ . '\I18nProfileField is deprecated in
   

   Let's put the trigger_error all on a single line, instead of multiples. That's what I see in the rest of core.

2. +++ b/core/modules/config_translation/src/Plugin/migrate/source/d6/I18nProfileField.php
   @@ -11,43 +13,12 @@
   +class I18nProfileField extends ProfileFieldTranslation {
    }
   

   Nit: the {} can be on the same line.

3. +++ b/core/modules/config_translation/tests/src/Kernel/Migrate/d6/MigrateSystemMaintenanceTranslationTest.php
   @@ -9,7 +9,7 @@
   -class MigrateI18nSystemMaintenanceTest extends MigrateDrupal6TestBase {
   +class MigrateSystemMaintenanceTranslationTest extends MigrateDrupal6TestBase {
   
   +++ b/core/modules/config_translation/tests/src/Kernel/Migrate/d6/MigrateSystemSiteTranslationTest.php
   @@ -9,7 +9,7 @@
   -class MigrateI18nSystemSiteTest extends MigrateDrupal6TestBase {
   +class MigrateSystemSiteTranslationTest extends MigrateDrupal6TestBase {
   
   +++ b/core/modules/config_translation/tests/src/Kernel/Migrate/d6/MigrateUserConfigsTranslationTest.php
   @@ -10,7 +10,7 @@
   -class MigrateI18nUserConfigsTest extends MigrateDrupal6TestBase {
   +class MigrateUserConfigsTranslationTest extends MigrateDrupal6TestBase {
   
   +++ b/core/modules/config_translation/tests/src/Kernel/Migrate/d6/MigrateUserProfileFieldInstanceTranslationTest.php
   @@ -9,7 +9,7 @@
   -class MigrateI18nUserProfileFieldInstanceTest extends MigrateDrupal6TestBase {
   +class MigrateUserProfileFieldInstanceTranslationTest extends MigrateDrupal6TestBase {
   

   On the old tests, let's add them to an @group of legacy.

   @group legacy

4. +++ b/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/i18nVariable.php
   @@ -2,10 +2,9 @@
   +@trigger_error('The ' . __NAMESPACE__ . '\i18nVariable is deprecated in
   +Drupal 8.4.0 and will be removed before Drupal 9.0.0. Instead, use
   +' . __NAMESPACE__ . '\VariableTranslation', E_USER_DEPRECATED);
   

   Same as earlier. A single line please.

Looks good now.

Needs a re-roll.

And back to RTBC.

Sorry to be "that guy," but what's the justification for changing these machine names around so late in the game? It looks cosmetic to me? Won't this break existing sites?

Yes it's a change we'd allow in a minor release even if migrate was fully stable since it's got a bc layer, although the issue title caught me out more than once when looking at this... It's handy that the bc layer for migrations is so simple by the way.

Re-reviewed the patch and it looks fine, so committed/pushed to 8.5.x and cherry-picked to 8.4.x. Thanks!