Problem/Motivation

MigrationInterface is in Entity tho they are not entities any more

Proposed resolution

Move the interface to migrate's Plugin namespace.

Note one reference to the old interface will be left - this will be fixed in #2687003: Remove references to nonexistent functionality from migrate.api.php as that whole api section needs adjusting for the plugins patch.

Remaining tasks

User interface changes

None

API changes

Move Drupal\migrate\Entity\MigrationInterface to Drupal\migrate\Plugin\MigrationInterface

Data model changes

None

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chx created an issue. See original summary.

alexpott’s picture

Issue summary: View changes

Given that the Migration entity no longer exists in core I think we should just move the interface out and be done.

alexpott’s picture

Issue summary: View changes
alexpott’s picture

I'm in favour of just moving the interface and being done.

heddn’s picture

Issue tags: +Novice

If we really are just moving the interface, then this can be marked novice.

alexpott’s picture

Status: Active » Needs review
FileSize
78.68 KB

@heddn whilst I agree this issue is novice I'd like to get the fallout from the plugins patch over asap.

benjy’s picture

Status: Needs review » Reviewed & tested by the community

Looks that got them all, RTBC. We can either add a new change record or I think we could probably get away with just adding to https://www.drupal.org/node/2668742

chx’s picture

I see no reason to do this. I already got enough flak from Xano about BC breaks. Convince him this is OK.

Rajab Natshah’s picture

Testing :)

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 10: 2676222-10.patch, failed testing.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

johnrosswvsu’s picture

Hi,

I worked on with @alexpott latest patch and amended the issue against 8.1.x and 8.2.x. Appending the patch for testing.

Removing lines for removed files and adding new ones for newly added files.

Please review.

Status: Needs review » Needs work

The last submitted patch, 13: drupal-core-proper-interface-for-migration-2676222-13.patch, failed testing.

johnrosswvsu’s picture

Status: Needs work » Needs review
FileSize
97.52 KB

With additional changes from https://www.drupal.org/node/2676222#comment-10939601. Aside from just renaming the interface, its uses, docblocks, and its namespaces I have to transfer the file also as one of the suggestions by @chx.

alexpott’s picture

This issue depends on #2625696: Make migrations themselves plugins instead of config entities we need to get that one done first - the point of this issue is to move the interface out of the entity namespace once migrations are no longer config entities.

alexpott’s picture

alexpott’s picture

Title: Add a proper interface for new Migration » Move MigrationInterface out of the migrate\Entity namspace now they are plugins
benjy’s picture

Just a couple more to fix.

grep -r 'migrate\\Entity' .
./modules/migrate/migrate.api.php:use Drupal\migrate\Entity\MigrationInterface;
./modules/migrate/migrate.api.php: * \Drupal\migrate\Entity\Migration, with interface
./modules/migrate/tests/modules/migrate_prepare_row_test/migrate_prepare_row_test.module:use Drupal\migrate\Entity\MigrationInterface;
alexpott’s picture

@benjy thanks - fixed all the code usages - going to fix the API section on Migration configuration entities in a separate patch.

alexpott’s picture

After discussion moved the interface to the Plugin directory to be consistent with the other migrate plugins. No interdiff because it would be as larger or maybe even larger than the patch.

The last submitted patch, 17: 2676222-2-17.patch, failed testing.

alexpott’s picture

Issue summary: View changes
benjy’s picture

Status: Needs review » Reviewed & tested by the community

Patch looks good, RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 21: 2676222-2-21.patch, failed testing.

alexpott’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
82.96 KB

Rerolled - All the conflicts were on deleted files due to #2676258: Nuke builder plugins and migration storage

  • core/modules/migrate/src/MigrationBuilder.php
  • core/modules/migrate/src/MigrationBuilderInterface.php
  • core/modules/migrate/src/Plugin/MigrateBuilderInterface.php
  • core/modules/migrate_drupal/src/Plugin/migrate/builder/CckBuilder.php
alexpott’s picture

xjm’s picture

Issue tags: -8.1.0 release notes
catch’s picture

Version: 8.2.x-dev » 8.1.x-dev

Just for the record:

I see no reason to do this. I already got enough flak from Xano about BC breaks. Convince him this is OK.

Tim Millwood asked me about the plugins patch in relation to bc as well in irc, because multiversion uses it. However I pointed him to https://www.drupal.org/core/d8-bc-policy and https://www.drupal.org/core/experimental which he was not aware of. multiversion is in alpha, so it's not really a problem for them to update to 8.1.x and drop support for 8.0.x.

If people want to discuss the bc policy there are issues to do that in, but this change is completely in-line with that. If it actually causes a problem for someone, we could add an empty extending interface for bc back to the entity namespace.

Committed/pushed to 8.2.x. Leaving RTBC against 8.1.x for a cherry-pick later.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Cherry-picked to 8.1.x

  • catch committed 1cc137f on 8.2.x
    Issue #2676222 by alexpott, johnrosswvsu: Move MigrationInterface out of...

  • catch committed 6b64cd9 on 8.1.x
    Issue #2676222 by alexpott, johnrosswvsu: Move MigrationInterface out of...

Status: Fixed » Closed (fixed)

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