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 byteskunalgautam
#2 3223725-2.patch852 bytesimalabya

Comments

quietone created an issue. See original summary.

imalabya’s picture

imalabya
Primary language English
Location Bangalore
commented
Status: Active » Needs review
StatusFileSize
new 3223725-2.patch852 bytes

Added a patch to remove the variable.

longwave’s picture

longwave
he/him
Primary language English
Location UK
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.

kunalgautam’s picture

kunalgautam commented
Status: Needs work » Needs review
StatusFileSize
new 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
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
commented
StatusFileSize
new interdiff.2-9.txt1.98 KB
new 3223725-9.patch1.83 KB
spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs work » Needs review
quietone’s picture

quietone 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
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.9-12.txt2.81 KB
new 3223725-12.patch4.62 KB

Is this what we're after?

quietone’s picture

quietone 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 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
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.