This module strives to deliver some extensions to the Entity Reference field from core.
Currently it provides additional Field Formatters for Entity Reference fields with multiple values, that allow to limit the entity reference items that will be displayed in different ways (first, last, offset, etc.)
Project link
https://www.drupal.org/project/entityreference_extensions
Git instructions
git clone --branch 8.x-1.x https://git.drupalcode.org/project/entityreference_extensions.git
PAReview
https://pareview.sh/pareview/https-git.drupal.org-project-entityreference_extensions.git
Comments
Comment #2
stefan.kornComment #3
Pinesh Kumar CreditAttribution: Pinesh Kumar commentedThank you for applying!
Please Check The PAReview Link.
Comment #4
stefan.kornOkay, I have shortened the doc comment a bit, because if split on two lines, there will be an error.
So now the warning in PAReview is gone.
Comment #5
C-LogemannI successfully integrated the module (8.x-1.0-beta4) in a test system to review it's functionality. It works fine at all. I only found a situation where expected another result. I created an issue for this: #3099072: Show all items except offset.
When creating this issue I recognized that the module currently has no "dev"-Release. So it's hard to test dev code for beginners and it's not possible to point on dev version on issues. So this should be added in my opion even if there is no rule for this.
I also did a short manual review of current dev code. I recognized was the entry "package: 'SK'" in info file. Even if there is no rule for creating own package sections for modules. As an admin I didn't like this small package groups on my module lists. Maybe better remove or add to a more common group like "Fields".
Currently I see no reason why @stefan.korn should no get „permission to opt into security advisory coverage”.
Comment #6
vuilThank you for the contribution!
I. In
Drupal\entityreference_extensions\EntityReferenceDeltaFilterTrait
:1. Please use
(int)
cast instead ofintval();
. It's 6x faster.2. You can also replace
with
3. Please replace
with
4. Replace also
with
II. Replace everywhere
t()
with$this->t()
which useStringTranslationTrait
.As an example: Replace
t('Show all')
with$this->t('Show all')
inDrupal\entityreference_extensions\Plugin\Field\FieldFormatter\EntityReferenceEntityDeltaFormatter
.Not all of the issues above are blockers, but it's better to use.
Comment #7
stefan.korn@vuil: Thanks for your review and suggestions. I have amended the code #3101790: Input from Project Application review.
Comment #8
vuil1. Please use simpler form of
instead of
2. Please also use
instead of the following (long) way
I didn't find a security related issue through the manual review, and I set the issue status to Reviewed & tested by the community.
Comment #9
apadernoComment #10
apadernoThank you for your contribution! I am going to update the project to opt it into security coverage.
These are some recommended readings to help with excellent maintainership:
You can find more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, also, for your patience with the review process.
Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.
I thank all the dedicated reviewers as well.
Comment #11
stefan.korn@kiamlaluno: Thanks for granting permission for security coverage on this project.
What is the process for having the possibility to opt in for security coverage for further (new) projects too? Reading [#1011698] I understand that only one project application per person is needed and requested? Have I missed something here?
Comment #12
apadernoOpen an application with a more complex project. Yes, normally it's necessary only a single application, if the code is not too simple.
Comment #13
stefan.kornThanks for clarification.
Is there any definition about a project that is complex enough to receive the full application?
If I want to get security coverage for some more rather simple projects will I need to apply for each one here separately?
Comment #15
apaderno@stefan.korn The next time you apply to get the vetted role, we will not look at the project complexity (which means it will not be a single application approval).