Migration class (from the migrate_drupal module not from the migrate module) variables should not be accessed directly. Functions should be use to access the variable. For instance use getDescription() and setDescription($description) for the protected class variable description. For a boolean variable the getter function becomes isVariableName(). In object-oriented programming this is called encapsulation.
Remaining tasks
- Update the class variables and make them protected.
- Create getters and setters for frequently used get and set functionality.
- Update drupal to use the getters and setters instead of accessing variables directly.
- There are no tests required because the added functions are only getters and setters.
For more info over what should be done see the issue summary of #2016679: Expand Entity Type interfaces to provide methods, protect the properties.
Beta phase evaluation
| Issue category | Bug because properties should not be public, API methods should not be allowed to be sidestepped. |
|---|---|
| Prioritized changes | Prioritized since it is a bug and it reduces fragility. |
| Disruption | Somewhat disruptive for core as well as contributed and custom modules:
|
But impact will be greater than the disruption, so it is allowed in the beta.
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | 2525002-1.patch | 468 bytes | daffie |
Comments
Comment #1
daffie commentedSee what happens when the only public class variable $load is set to protected.
Comment #2
benjy commentedLooks good to me.
Comment #3
daffie commentedTalked with benjy on IRC. The variable load is to be removed as are the load plugins. Because the variable is not used outside the class there are no getter and setter methods necessary. Benjy does not think the a change record is necessary.
Comment #4
xjmI confirmed the only use of this property is internal to the class. Agreed that we probably do not need a change record for this.
This issue only changes Migrate code, so per https://www.drupal.org/core/beta-changes, this can be completed any time during the Drupal 8 beta phase. Committed and pushed to 8.0.x. Thanks!
Comment #7
xjmYes, it doesn't apply this time, because it's in HEAD. Thanks testbot. :P