Problem/Motivation

MigrateCckFieldInterface is deprecated in Drupal 8.3.x and will be removed before Drupal 9.0.x.

Proposed resolution

Use \Drupal\migrate_drupal\Annotation\MigrateField instead.

Remaining tasks

User interface changes

API changes

Data model changes

Comments

mihai11 created an issue. See original summary.

mihai11’s picture

Assigned: mihai11 » Unassigned
Status: Active » Needs review
StatusFileSize
new2.56 KB

Status: Needs review » Needs work

The last submitted patch, 2: 2969902.patch, failed testing. View results

mihai11’s picture

Status: Needs work » Needs review
StatusFileSize
new1.89 KB
new2.53 KB

Update after test fail.

The last submitted patch, 2: 2969902.patch, failed testing. View results

amateescu’s picture

Status: Needs review » Needs work
+++ b/core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php
@@ -143,6 +143,7 @@ public static function getSkippedDeprecations() {
+      'MigrateCckFieldInterface is deprecated in Drupal 8.3.x and will be removed before Drupal 9.0.x. Use \Drupal\migrate_drupal\Annotation\MigrateField instead.',

This part of the interdiff is not correct, we shouldn't add back the deprecation message to the trait :)

amateescu’s picture

Issue tags: -DCTransylvania2018

Cleaning up tags.

mihai11’s picture

Status: Needs work » Needs review
StatusFileSize
new3.5 KB
new1.6 KB

Removed the deprecation message from the trait.

Status: Needs review » Needs work

The last submitted patch, 8: 2969902-8.patch, failed testing. View results

mihai11’s picture

Status: Needs work » Needs review
StatusFileSize
new3.54 KB

Rerolled the patch.

amateescu’s picture

Status: Needs review » Reviewed & tested by the community

Looks great :)

borisson_’s picture

I was just reviewing this issue as well, I came to the same conclusion as @amateescu! RTBC++ :)

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/link/tests/src/Kernel/Plugin/migrate/cckfield/d7/LinkCckTest.php
@@ -54,7 +54,6 @@ protected function setUp() {
-   * @expectedDeprecation MigrateCckFieldInterface is deprecated in Drupal 8.3.x and will be removed before Drupal 9.0.x. Use \Drupal\migrate_drupal\Annotation\MigrateField instead.

It's super strange to be removing an @expectedDeprecation.

+++ b/core/modules/migrate_drupal/src/Plugin/migrate/cckfield/CckFieldPluginBase.php
@@ -18,7 +18,7 @@
-abstract class CckFieldPluginBase extends FieldPluginBase implements MigrateCckFieldInterface {
+abstract class CckFieldPluginBase extends FieldPluginBase implements MigrateFieldInterface {

This class is deprecated too. It's okay for deprecated code to use deprecated code :)

Can we get a patch that just removes the deprecation?

amateescu’s picture

@alexpott, I also saw this comment in another issue, so I'm wondering: when a patch makes a test method not trigger a deprecation message anymore, shouldn't that patch also remove the @expectedDeprecation annotation?

alexpott’s picture

Not if the test is a legacy test. legacy tests are there to test deprecated code and will be removed at the same time as deprecated code.

amateescu’s picture

Right, that makes sense!

jofitz’s picture

Status: Needs work » Needs review
StatusFileSize
new1.89 KB
new1.65 KB

Creating the patch requested by @alexpott in #13.

Status: Needs review » Needs work

The last submitted patch, 17: 2969902-17.patch, failed testing. View results

jofitz’s picture

Status: Needs work » Needs review
StatusFileSize
new542 bytes
new2.18 KB

Set CckFieldBackwardsCompatibilityTest as a legacy test.

alexpott’s picture

Status: Needs review » Needs work

@Jo Fitzgerald can you add @expectedDeprecation too?

jofitz’s picture

Status: Needs work » Needs review
StatusFileSize
new849 bytes
new2.62 KB

Added @expectedDeprecation.

Anonymous’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +DCScot18

I have reviewed this patch and it looks good to me :-)

Status: Reviewed & tested by the community » Needs work

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

Mixologic’s picture

Status: Needs work » Reviewed & tested by the community

Testbot Snafu.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed beae7599dc to 8.6.x and 71ae9121fc to 8.5.x. Thanks!

Credited @amateescu and myself for reviews that materially affected the patch. Thanks @davemckain and @borisson_ also for the reviews.

  • alexpott committed beae759 on 8.6.x
    Issue #2969902 by mihai11, Jo Fitzgerald, amateescu, alexpott: Fix "...

  • alexpott committed 71ae912 on 8.5.x
    Issue #2969902 by mihai11, Jo Fitzgerald, amateescu, alexpott: Fix "...

Status: Fixed » Closed (fixed)

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