Problem/Motivation
In working on Ubercart 7 to Commerce 2 I came across a reason to extend Field.php but it is marked @internal. The reason stems from the fact that products are nodes in Ubercart 7 but commerce_products in Commerce 2. If a field exists on a product node and another non product node type the Commerce 2 sites needs field storage on the node entity and the commerce_product entity. To do this adding a new row to the field plugin mimicking that the field existed in the source on a commerce_product entity works well.
Field.php was marked internal in #2891932: Refactor D7 Field source plugin and the references to the decision are in comments in #10 and #11. It states that @internal was used to keep some flexibility if this needed to be changed in the future.
The plugin was refactored in 8.4.x, there are have been 3 changes to the file since then, 2 for fixes to the annotation and 1 to add handling of Drupal 7 title data.
Proposed resolution
Since nothing has turned up yet to indicate that this needs to remain internal, let's remove it.
Remaining tasks
Patch that removes the following for d7/Field.php
* @internal
*
* This class is marked as internal and should not be extended. Use
* Drupal\migrate_drupal\Plugin\migrate\source\DrupalSqlBase instead.
*
Review
Commit
API changes
The source plugin is no longer @internal.
Marking novice for making the initial patch as described in 'Remaining Tasks'.
Comment | File | Size | Author |
---|---|---|---|
#5 | 2996012-5.patch | 1.23 KB | anmolgoyal74 |
#2 | 2996012-2.patch | 603 bytes | vakulrai |
Comments
Comment #2
vakulrai CreditAttribution: vakulrai as a volunteer and at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commentedAdding a patch for the same.
Comment #3
maxocub CreditAttribution: maxocub commentedShould we do the same for Drupal\field\Plugin\migrate\source\d7\FieldInstance for consistency's sake?
Comment #4
heddnYeah, let's do it for field instance too. Back to NW and still leaving the novice tag as that should be pretty strait forward.
Comment #5
anmolgoyal74 CreditAttribution: anmolgoyal74 at OpenSense Labs for DrupalFit commentedComment #6
maxocub CreditAttribution: maxocub commentedThis looks good, thanks!
Comment #8
th_tushar CreditAttribution: th_tushar as a volunteer commentedMarking it as RTBC, as patch cleared testing.
Comment #9
catchCommitted c3e26c4 and pushed to 8.7.x. Thanks!