This is a really, really minor thing...but it bugs the hell out of me and it has been for a while. Can we, like, give Migrate's Row class default constructor arguments so that in testing we don't always have to write new Row([], [])? Seriously minor...but it drives me crazy. If it annoys anybody else, by all means chime in so I don't feel like a weird neat freak.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

phenaproxima created an issue. See original summary.

phenaproxima’s picture

Status: Active » Needs review
FileSize
10.74 KB
quietone’s picture

Simple this may be but it does help. Plus, I reckon helping phenaproxima's to keep his sanity is a good thing.

New patch because after applying the patch grep reported an instance of "new Row([], [])".

phenaproxima’s picture

benjy’s picture

Status: Needs review » Reviewed & tested by the community

This is a funny one, Row is an internal object to Migrate, nobody should be instantiating their own so I can't see any issues with this.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed d11cc0a to 8.3.x and 0cf889e to 8.2.x. Thanks!

And even if someone was using it - supplying defaults is not going to break a thing. The signature has not really changed.

diff --git a/core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityContentBaseTest.php b/core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityContentBaseTest.php
index c9b3d11..bbd134c 100644
--- a/core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityContentBaseTest.php
+++ b/core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityContentBaseTest.php
@@ -95,7 +95,7 @@ public function testImportEntityLoadFailure() {
       $this->entityManager->reveal(),
       $this->prophesize(FieldTypePluginManagerInterface::class)->reveal());
     $destination->setEntity(FALSE);
-    $destination->import(new Row);
+    $destination->import(new Row());
   }
 
   /**

Fixed on commit.

  • alexpott committed d11cc0a on 8.3.x
    Issue #2808329 by quietone, phenaproxima: Add default values to Row...

Status: Fixed » Closed (fixed)

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