Problem/Motivation

Migrate as committed contained both framework and D6 source specific bits.

Proposed resolution

Move the D6 source into a migrate_drupal module.

CommentFileSizeAuthor
#5 interdiff-4-5.txt4.05 KBbdone
#5 2143405-5.patch37.34 KBbdone
#3 2143405_3.patch14.01 KBchx
migrate_split.patch46.05 KBchx

Comments

chx’s picture

chx commented
Status: Active » Needs review

BTW thanks for committing #2132441: run-tests.sh --module is broken makes my life quite a bit easier.

chx’s picture

chx commented

Note that we have our first config entity migration which I will submit the moment this is in and we have rapidly expanding testing for the Sql and MigrateExecutable classes and... aaaaargh it's going to be very hard to keep up with the pace we have, I wonder how the VDC team did this.

chx’s picture

chx commented
Title: Migrate patch #2: small fixes and split to migrate_drupal » Split migrate into to migrate and migrate_drupal
Issue summary: View changes
StatusFileSize
new 2143405_3.patch14.01 KB
dawehner’s picture

dawehner
Primary language German
commented

This comment feels so dirty.

+++ b/core/modules/migrate_drupal/config/migrate.migration.d6_system_performance.yml
@@ -0,0 +1,14 @@
+    plugin: drupal6_variable
+    variables:

Just wonder why we use 4 spaces instead of 2 spaces everywhere else.

bdone’s picture

bdone commented
StatusFileSize
new 2143405-5.patch37.34 KB
new interdiff-4-5.txt4.05 KB

hopefully less dirty

chx’s picture

chx commented

Because
protected $indentation = 4;

in Symfony Yaml Dumper. Reconfiguring migrate to use two spaces is doable but an entirely different issue than moving files around.

chx’s picture

chx commented

bdone prodded me enough that the sandbox now has spaces reconfigured but it's still a different issue.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs review » Reviewed & tested by the community

Looks good, changing the indents will break the clean renames and can be tackled in bulk elsewhere.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Will the new migrate_drupal module also contain the Drupal 7 and 8 etc... migration classes?

chx’s picture

chx commented

Yes for D7. As for D8 sources, who knows. We need to discuss that. I suspect yes but I am not entirely sure.

D8 destinations, no, that's part of migration, while theoretically you can migrate into anything most people will want to migrate into D8 and so we will keep the D8 destinations in migrate.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs review

Sorry files in #6 are hidden, so is #5 or #6 RTBC?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs review » Reviewed & tested by the community

#3 contains the rtbc patch #5 changed the spacing of some yaml files that was out of scope.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

chx’s picture

chx commented
Parent issue: » #2121299: Migrate in Core: Drupal 6 to Drupal 8

Status: Fixed » Closed (fixed)

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