The Advanced Datalayer provides flexible possibility to manipulate datalayer
page variables https://developers.google.com/tag-manager/devguide.
This module is API module and provides base plugins code and admin part
to create and manipulate your own datalayer page variables (tags).
Basic approach, inherit logic and architecture are similar to the metatag module.

Project link

https://www.drupal.org/project/advanced_datalayer

Git instructions

git clone --branch 1.x https://git.drupalcode.org/project/advanced_datalayer.git

PAreview checklist

http://pareview.net/r/329

Comments

Kenniec created an issue. See original summary.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Priority: Normal » Critical

I changed the issue priority as described on Review process for security advisory coverage: What to expect / Application Review Timelines.

To reviewers: Please change back the priority to Normal after reviewing the project.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Priority: Critical » Major
marijan gudelj’s picture

marijan gudelj
Primary language Croatian
commented

Pareview.net
failed
http://pareview.net/r/329

FILE: ...viewd/pareview_temp/zanbfjee/src/Annotation/AdvancedDatalayerTag.php
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
61 | ERROR | Class property $show_empty should use lowerCamel naming
| | without underscores
--------------------------------------------------------------------------
marijan gudelj’s picture

marijan gudelj
Primary language Croatian
commented
Priority: Major » Normal
Status: Needs review » Needs work

Automated review failed. Switching back to normal priority and needs work

kenniec’s picture

kenniec commented
Status: Needs work » Needs review

Hi @marijan-gudelj,

Thank you for review,

but this is false-positive problem, seems on http://pareview.net used old version of coder.

According drupal coding standards ( https://www.drupal.org/node/608152#naming)

2. Methods and class properties should use lowerCamel naming. In Drupal 8, properties of configuration entities are exempt of these conventions. Those properties are allowed to use underscores.

Also, see coder issue with fix for this - https://www.drupal.org/project/coder/issues/2804739

BR,
Alex

marijan gudelj’s picture

marijan gudelj
Primary language Croatian
commented

I would like that apadermo also gives me his input about the naming.
When you look at the naming in major drupal modules (Example Commerce/Cart) property names are camelCase.
My guess that is because before snake case was used.

Regarding the source files in your module:
Is the context_advanced_datalayer a submodule?
Is the example_advanced_datalayer a submodule?

kenniec’s picture

kenniec commented

Hi @marijan-gudelj,

in last version of Commerce - https://www.drupal.org/project/commerce, in annotation files you can also see properties with underscores
Drupal\commerce\Annotation\CommerceEntityTrait
Drupal\commerce\Annotation\CommerceCondition

also, in d9 core like example
Drupal\Core\Field\Annotation\FieldWidget
Drupal\Core\Field\Annotation\FieldFormatter

all this classes - annotations, like my file from Pareview.net report (/src/Annotation/AdvancedDatalayerTag.php).

context_advanced_datalayer - yes, submodule to integrate with Context module (https://www.drupal.org/project/context).
example_advanced_datalayer - yes, submodule, some examples of group/tag plugins.

BR,
Alex

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented

Annotation classes use property names containing underscores, there are many example in Drupal core, for example the MediaSource class. That is allowed from Drupal coding standards.

marijan gudelj’s picture

marijan gudelj
Primary language Croatian
commented

@Kenniec I would suggest that you place your submodules in a modules directory.
I will do a full manual review soon.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Assigned: Unassigned » avpaderno
Status: Needs review » Reviewed & tested by the community
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Reviewed & tested by the community » Fixed

Thank you for your contribution! I am going to update your account.

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.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.