Problem/Motivation
I found two incorrect type declarations in migrate module:
1. Class property $idMap in \Drupal\migrate\Plugin\Migration declared as string while correct type is array.
2. Class property $result in \Drupal\migrate\Plugin\migrate\id_map\Sql declared as null while correct type is \Drupal\Core\Database\StatementInterface|null
Steps to reproduce
none
Proposed resolution
Fix type declarations.
Remaining tasks
Create merge request.
User interface changes
none
API changes
none
Data model changes
none
Release notes snippet
Issue fork drupal-3212891
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
vaish CreditAttribution: vaish for Greentech Renewables commentedComment #4
Akhildev.cs CreditAttribution: Akhildev.cs as a volunteer and at Zyxware Technologies commentedhi,
I tried patch #2 manually and for me, the patch applied successfully.
working fine, Thanks
Comment #5
quietone CreditAttribution: quietone as a volunteer commented@Akhildev.cs, Welcome to Drupal! I am not sure what you mean when you say that you tried the patch manually. This patch changes documentation, not code, so there is nothing to 'try' here. When patches or Merge Requests are tested by the testbot we know that it applies successfully, there is no need to make a comment about that (it adds noise to the issue). The Drupal community has a wealth of information for Getting started with contributing. You may find Review a patch or merge request of particular interest.
Cheers
Comment #6
mikelutzThis seems fine, though I notice that There is no way to set the value of Migration::idMap to anything but an empty array. I suppose it may be set by subclasses, so I'm inclined to leave it for now.
Simple doc change only, so RTBC
Comment #7
alexpottI added a review to the MR.
Comment #8
quietone CreditAttribution: quietone as a volunteer commentedThe changes here are suitable for a novice.
Comment #9
MatroskeenFixing the tag :)
Comment #11
brentgUpdated the MR with the review from Alex
Comment #12
mikelutzStill need to remove the |null from the docblock for $result per @alexpott
Comment #14
srilakshmier CreditAttribution: srilakshmier at Valuebound for Valuebound commentedHi, removed |null from the docblock for @result as per #12. Please review.
Thank you
Comment #15
mikelutzTY!
Comment #16
alexpottCommitted and pushed 2aee2c17b3 to 9.4.x and 8182c97eb8 to 9.3.x. Thanks!