add automated quality control & post-commit script

[WengerK]

This PR make a lot of changes here why:

Automated Quality Control - Pareview

I added a new Quality Control script which is the same as https://pareview.sh/ for Drupal.

You can use this scripts by:

1. Install depencency

composer install

2. Add the post-commit script in your .git

cat ./scripts/hooks/post-commit >> ./.git/hooks/post-commit

3. Commit or use the script

./.git/hooks/post-commit

This script will run:

• phpcs: Code Sniffer Drupal & DrupalPractice
• phpmd: PHP Mess Detector
• phpcpd: PHP Copy/Paste Detector
• phpcf: PhpCodeFixer

Refactoring to follow Drupal Coding Standard instead of Symfony ones

Also, by adding those, I had to change a lot of code - essential for the following reasons:

• The code follows the Symfony Best Pratices (most of the time because variable's name was camelCase instead of snake_case @see https://www.drupal.org/docs/develop/standards/coding-standards
• The code miss some comments (@wanze it could be great if you could review because I "sometimes" added some comment whiteout having a deep knowledge of what was the purpose of some feature/method/property).

[WengerK]

Do you agree with this change ?

The first comment must be a one line comment, the second can be multiline (drupal 8 coding standard).

[wanze]

Maybe we could rephrase the first line to make it more clear, something like "Event to map a commerce product to a GTM product". It is really not easy given the limited characters available :)

[WengerK]

Yeah it's quite a challenge.

What about this ?

  /**
   * Allows to alter the GTM Product field mapping.
   *
   * Allows alteration of field mapping from Commerce product to a GTM Product.
   * Use this event to alter data Product before it gets pushed to data layer.

@wanze

[wanze]

@WengerK Thank you for setting up the code quality tools and the excellent docs! 👍

I left some nitpicking comments about some code comments ;-) One question regarding the Drupal coding standards. The docs leave it open to use camelCase or snake_case for variable names. Now we are mixing both styles, e.g. $this->eventTrackerService = $event_tracker_service. I know that most Drupal modules use snake_case (I personally prefer camel) but I think we should be consistent. What do you think?

The amount of unnecessary comments we have to write (anti-pattern) is another topic, but I know things are how they are... :)

[wanze]

Maybe we could rephrase the first line to make it more clear, something like "Event to map a commerce product to a GTM product". It is really not easy given the limited characters available :)

[WengerK]

Thanks for the review !

The amount of unnecessary comments we have to write (anti-pattern) is another topic, but I know things are how they are... :)

Unfortunately, the coding standard request a comment on every method :(. This is necessary for the automatic documentation generator.

One question regarding the Drupal coding standards. The docs leave it open to use camelCase or snake_case for variable names. Now we are mixing both styles, e.g. $this->eventTrackerService = $event_tracker_service. I know that most Drupal modules use snake_case (I personally prefer camel) but I think we should be consistent. What do you think?

I absolutely agree to be consistent; do not mix camelCase and snake_case variable naming inside a file.

But, the naming convention works as follow:

• Classes and interfaces should use UpperCamel naming.
• 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.
• Functions should be named using lowercase, and words should be separated with an underscore.
• Constants should always be all-uppercase, with underscores to separate words.

So the only "black-point" is about the variables that can be both (camel or snake).
I would prefer to use snake case as all the Core is done following this principle.

Here another doc about this: https://www.drupal.org/docs/develop/standards/object-oriented-code
Also an interesting concern for the Drupal community which often disallow lowerCamelCase for variables: https://www.drupal.org/project/coding_standards/issues/2648050
#36 is very instructive

I can live with both for variables (whitout mixing of course) so If you prefer to switch variable to lowerCamelCase just 🎉 this comment & I will operate the changes at the same time I work on all others typos & suggestions 👍

PS: I will work on the change you request later this day ^^

[WengerK]

Add missing \ on comment for automatic documentation & IDE autocomplete

[wanze]

@WengerK Thanks for the explanations!

I would prefer to use snake case as all the Core is done following this principle.

I agree, let's follow the principles from Drupal core 👍

[WengerK]

Add suggestions to adapte "variation title" into a more generic "variation"