Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Review code in relation to D8 and Search API standards.
Comment | File | Size | Author |
---|---|---|---|
#2 | module-as-patch.patch | 16.21 KB | borisson_ |
Comments
Comment #2
borisson_Uploading module as a patch so I can comment easier without having to make an actual patch.
Comment #3
borisson_Overall this is looking very nice! Adding tests is of course a good idea, but you already have another issue open for this.
I did find some other things that can be changed, most things are coding standards related.
This shouldn't be in the repo here. I don't see any other modules that have a gitignore.
Some line endings in this file have spaces. Those should be removed.
It makes it possible to exclude entities from being indexed in a Search API index.
I don't think this is needed, the drupal composer facade already creates a composer file.
I haven't seen any documentation about this, but as far as I know the best practice is to namespace dependencies. In this case that's drupal:field, search_api:search_api.
This is not needed.
needs eof newline
Needs eof newline.
@file docblocks are not longer needed for files that contain only one class.
@file docblocks are not longer needed for files that contain only one class.
@file docblocks are not longer needed for files that contain only one class.
@file docblocks are not longer needed for files that contain only one class.
I think this could use a docblock title.
Proper DI here would be great, use
::create
to inject the manager so we don't have to call\Drupal::service
from here.and here.
return isset($field_map[$entity_type][$field]['bundles'][$bundle])
is the same thing.This comment should be reflowed to be closer to 80 cols.
The coding standards don't allow yoda if statements. The NULL should be on the other side of the
!==
Comment #6
beltofteThanks @borisson_ for the nice code review!
I have applied all the suggestions and committed the changes.