Problem/Motivation

This is a follow up to #2917883: Rename 'migration_templates' directories in core modules to 'migrations'. See #20

There are migrations in core that are prefixed with "migrate.migration" and that is not needed. It might be confused with migrations configured with migrate_plus which uses a prefix of 'migrate_plus.migration' and it is just extra typing as well.

Using find these files were discovered.

./core/modules/migrate/tests/modules/migrate_high_water_test/migrations/migrate.migration.high_water_test.yml
./core/modules/migrate/tests/modules/migrate_external_translated_test/migrations/migrate.migration.external_translated_test_node.yml
./core/modules/migrate/tests/modules/migrate_external_translated_test/migrations/migrate.migration.external_translated_test_node_translation.yml

Suitable for novice

Proposed resolution

Rename all such migration yml files to remove the prefix "migrate.migration."

Remaining tasks

Write a patch
Review
RTBC
Commit!

User interface changes

N/A

API changes

N/A

Data model changes

N/A

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quietone created an issue. See original summary.

quietone’s picture

Issue tags: +Novice

Added novice tag.

shashikant_chauhan’s picture

Status: Active » Needs review
FileSize
5.3 KB

patch for renaming files.

quietone’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

@shashikant_chauhan, thanks for making the patch. It is nearly there!

+++ /dev/null
@@ -1,27 +0,0 @@
diff --git a/core/modules/migrate/tests/modules/migrate_high_water_test/migration_templates/high_water_test.yml b/core/modules/migrate/tests/modules/migrate_high_water_test/migration_templates/high_water_test.yml

Where you working from HEAD? The high_water_test.yml was moved from migration_templates to migrations directory.

When making this patch please use 'git diff -M' and check that the patch uses rename and does delete the file and then insert a new one. It just makes it much easier to review. Thanks!

Ivan Berezhnov’s picture

Issue tags: +CSKyiv18
savkaviktor16@gmail.com’s picture

Re-rolled/remade the patch

savkaviktor16@gmail.com’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
quietone’s picture

Status: Needs review » Reviewed & tested by the community

Thanks for the reroll and having it show renames. So much easier to review. Applied the patch and used find and did not find any files with the name 'migrate.migration' prepended to it.

Thanks Saviktor and shashikant_chauhan !

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: 2920963-6.patch, failed testing. View results

andypost’s picture

Status: Needs work » Reviewed & tested by the community

Requeued, unrelated fail

xjm’s picture

Title: Remove the migrate.migration prefix from any core migrations » Remove the migrate.migration prefix from core test migrations

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: 2920963-6.patch, failed testing. View results

quietone’s picture

Retesting

quietone’s picture

Status: Needs work » Reviewed & tested by the community

Tests are passing again. Back to RTBC

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: 2920963-6.patch, failed testing. View results

quietone’s picture

Status: Needs work » Needs review

Once again, the test aborted. Let's try again.

quietone’s picture

Status: Needs review » Reviewed & tested by the community

Back to RTBC

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: 2920963-6.patch, failed testing. View results

jofitz’s picture

Status: Needs work » Reviewed & tested by the community

Tests are back to green. Back to RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: 2920963-6.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 21: 2920963-6.patch, failed testing. View results

D34dMan’s picture

Regarding the failing test cases, added the failed test as reference to the Drupal issue which is tracking it #2926309: Random fail due to APCu not being able to allocate memory

D34dMan’s picture

Status: Needs work » Reviewed & tested by the community

After doing the retest we have two fails which are irrelevant to this issue as mentioned by @andypost in #10. Thus marking it as RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 21: 2920963-6.patch, failed testing. View results

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

quietone’s picture

Status: Needs work » Needs review

The random failures have been fixed, #2926309: Random fail due to APCu not being able to allocate memory, and this is passing tests on 8.6.x. This is just renaming three files so let's go to NR.

phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

I'm a bit concerned about the PHP 7.2 failures, but I know this patch didn't cause them. I think this is ready to go.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

@phenaproxima those fails were from before PHP 7.2 was actually supported :)

Committed and pushed dd993b38f6 to 8.6.x and 50f0a25035 to 8.5.x. Thanks!

Backported to 8.5.x as this is a test-only change.

  • alexpott committed dd993b3 on 8.6.x
    Issue #2920963 by quietone, Saviktor, shashikant_chauhan: Remove the...

  • alexpott committed 50f0a25 on 8.5.x
    Issue #2920963 by quietone, Saviktor, shashikant_chauhan: Remove the...

Status: Fixed » Closed (fixed)

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