rlmumford to review GDPR fields module #11
Conversation
rlmumford
left a comment
rlmumford
left a comment
There was a problem hiding this comment.
It looks like >60% of this is due to be re-written on switching to entity wrappers. There's also of functionality breaking debugging code and empty functions left in.
| @@ -0,0 +1,7 @@ | |||
| <?php | |||
There was a problem hiding this comment.
Do we need this file if it is empty?
| 'length' => 32, | ||
| 'description' => 'Entity bundle of field.', | ||
| ), | ||
| 'field_name' => array( |
There was a problem hiding this comment.
Consider changing to word 'property' as I understand this is more about properties than fields.
| ); | ||
|
|
||
| return $schema; | ||
| } No newline at end of file |
| * @return array | ||
| * An array of arrays with information about all available panel contexts. | ||
| */ | ||
| function gdpr_fields_get_contexts() { |
There was a problem hiding this comment.
This function isn't doing anything - remove.
Also never called.
| /** | ||
| * Implements hook_menu(). | ||
| */ | ||
| function gdpr_fields_menu() { |
| <?php | ||
|
|
||
| /** | ||
| * Base class for export UI. |
There was a problem hiding this comment.
This comment doesn't look right to me.
| var $options = array(); | ||
|
|
||
| /** | ||
| * Fake constructor. |
There was a problem hiding this comment.
This is not a constructor and not fake?
| } | ||
|
|
||
| function gdpr_fields_gdpr_entity_field_get_children($plugin, $parent) { | ||
| $instances = field_info_instances(); |
| * @file | ||
| * Contains the gdpr_entity_property plugin definition. | ||
| */ | ||
| $plugin = array( |
There was a problem hiding this comment.
@yanniboi is there a reason that these are split into two different plugins?
| @@ -0,0 +1,254 @@ | |||
| <?php | |||
There was a problem hiding this comment.
@yanniboi This looks like it would be really useful elsewhere but not here if we're switching to properties. Is there any other need for this that I am missing?
|
I'm pretty sure Andrews commit adds a functional error. It removes the variable $is _sub_list because it was unused. The variable need to be there and the if statement a few lines below needs to test is_sub_list rather than is_list |
No description provided.