Problem/Motivation

Followup from #2796755: [PP-1] Deprecate Migration::set().

In comment #124 alexpott said "We need to remove \Drupal\migrate\Plugin\Migration::$dependencies it is unused and confusing".

Steps to reproduce

N/A

Proposed resolution

Remove unused variable $dependencies

Remaining tasks

Patch
Review
Commit

User interface changes

N/A

API changes

N/A

Data model changes

N/A

Release notes snippet

N/A

CommentFileSizeAuthor
#12 3223725-12.patch4.62 KBSpokje
#12 interdiff.9-12.txt2.81 KBSpokje
#9 3223725-9.patch1.83 KBSpokje
#9 interdiff.2-9.txt1.98 KBSpokje
#7 removed_unused_var-3223725-7.patch905 byteskkalashnikov
#2 3223725-2.patch852 bytesimalabya
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quietone created an issue. See original summary.

imalabya’s picture

imalabya
he/him
Primary language English
Location Bangalore
CreditAttribution: imalabya at Specbee for Drupal India Association commented
Status: Active » Needs review
FileSize
3223725-2.patch852 bytes

Added a patch to remove the variable.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs review » Needs work

TestMigrationMock in MigrationPluginManagerTest extends this class and uses this property, we should change this to use migration_dependencies I think.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

kkalashnikov’s picture

kkalashnikov CreditAttribution: kkalashnikov as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
Status: Needs work » Needs review
FileSize
removed_unused_var-3223725-7.patch905 bytes

Re-roll patch for Drupal version 10.1.x

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs review » Needs work

There was no need to reroll the patch, #2 still applies, so removing credit for #7. #3 still needs addressing, the CI failed now we have PHPStan to pick up on this type of thing.

Spokje’s picture

Spokje
Primary language Dutch
CreditAttribution: Spokje as a volunteer commented
FileSize
interdiff.2-9.txt1.98 KB
3223725-9.patch1.83 KB
Spokje’s picture

Spokje
Primary language Dutch
CreditAttribution: Spokje as a volunteer commented
Status: Needs work » Needs review
quietone’s picture

quietone CreditAttribution: quietone at PreviousNext commented
Status: Needs review » Needs work

In the test file, is there a reason to limit the change to the test mock? I don't see why. So, I think we should also change it in testDependencyBuilding and dependencyProvider.

Spokje’s picture

Spokje
Primary language Dutch
CreditAttribution: Spokje as a volunteer commented
Status: Needs work » Needs review
FileSize
interdiff.9-12.txt2.81 KB
3223725-12.patch4.62 KB

Is this what we're after?

quietone’s picture

quietone CreditAttribution: quietone at PreviousNext commented
Title: Remove unused variable \Drupal\migrate\Plugin\Migration::$dependencies » Remove unused class property \Drupal\migrate\Plugin\Migration::$dependencies

Make it clear that this is different from other issues that are removing an unused variable.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

As the task seems to be just to remove the variable that appears to be complete (updated issue summary too).

  • catch committed 2fbac34e on 10.1.x 
    Issue #3223725 by Spokje, imalabya, quietone, longwave: Remove unused...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed 2fbac34 and pushed to 10.1.x. Thanks!

Status: Fixed » Closed (fixed)

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