[8.x-1.x] Codepen

Will apply later, this can be deleted.

Ok, had to do some checks in Drupal 10, did want to rely on the automatic update only. It turned out to be OK.

Thank you for applying! Reviewers will review the project files, describing what needs to be changed.

Please read Review process for security advisory coverage: What to expect → for more details and Security advisory coverage application checklist → to understand what reviewers look for. Tips for ensuring a smooth review gives some hints for a smoother review.

To reviewers: Please read How to review security advisory coverage applications → , What to cover in an application review → , and Drupal.org security advisory coverage application workflow → .

While this application is open, only the user who opened the application can make commits to the project used for the application.

Reviewers only describe what needs to be changed; they don't provide patches to fix what reported in a review.

• The following points are just a start and don't necessarily encompass all of the changes that may be necessary
• A specific point may just be an example and may apply in other places
• A review is about code that doesn't follow the coding standards, contains possible security issue, or doesn't correctly use the Drupal API; the single points aren't ordered, not even by importance

codepen.info.yml

# version: VERSION
core: 8.x
core_version_requirement: ^8 || ^9 || ^10

version is only used by Drupal core modules. core cannot be used with that value of core_version_requirement.

src/Form/CodepenSettingsForm.php

$config = $this->configFactory->get('codepen.settings');

There is no need to use $this->configFactory(), in a class that extends ConfigFormBase.

    $form['codepen_global']['codepen_theme'] = [
      '#type' => 'checkbox',
      '#title' => $this->t('Enable light theme'),
      '#default_value' => $config->get('codepen_theme'),
      '#description' => $this->t('Themes allow you to style Embedded Pens. Need to create or edit your themes? Go to the <a href="https://codepen.io/embed-theme-builder">Embed Theme Builder</a>.'),
    ];

URLs are added to translatable strings using placeholders.

src/Plugin/Field/FieldFormatter/CodepenFormatter.php

  /**
   * {@inheritdoc}
   */
  public function prepareView(array $entities_items) {}

There is no need to implement an empty method.

src/Plugin/Field/FieldWidget/CodepenDefaultWidget.php

  /**
   * Validate Codepen URL.
   */

The documentation comment does not document the method parameters nor the returned value.

The LICENSE file isn't necessary, since every project hosted on drupal.org is licensed under the same license used by Drupal core. That file is also automatically added from the packaging script.

Fix Drupal Coding Standard issues. You can use the PHPCS tool for checking and resolving issues.

phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml codepen/

FILE: codepen/codepen.module
--------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
--------------------------------------------------------------------------
  43 | ERROR | [x] Missing function doc comment
 130 | ERROR | [ ] join() is a function name alias, use implode() instead
 152 | ERROR | [x] Expected 1 newline at end of file; 2 found
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: codepen/src/Feeds/Target/CodepenItem.php
------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
------------------------------------------------------------------------------
 39 | WARNING | [ ] Line exceeds 80 characters; contains 87 characters
 46 | ERROR   | [x] Concat operator must be surrounded by a single space
------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------


FILE: codepen/src/Plugin/Field/FieldFormatter/CodepenFormatter.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 66 | ERROR | The array declaration extends to column 109 (the limit is 80). The array content should be split up over multiple lines
--------------------------------------------------------------------------------


FILE: codepen/src/Plugin/Field/FieldWidget/CodepenDefaultWidget.php
--------------------------------------------------------------------------------
FOUND 5 ERRORS AFFECTING 5 LINES
--------------------------------------------------------------------------------
  73 | ERROR | [x] Use null coalesce operator instead of ternary operator.
  85 | ERROR | [x] Use null coalesce operator instead of ternary operator.
  91 | ERROR | [x] Use null coalesce operator instead of ternary operator.
  97 | ERROR | [x] Use null coalesce operator instead of ternary operator.
 103 | ERROR | [x] Use null coalesce operator instead of ternary operator.
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 5 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: codepen/tests/src/Functional/CodepenTest.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 4 WARNINGS AFFECTING 4 LINES
--------------------------------------------------------------------------------
 122 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 124 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 139 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 141 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
--------------------------------------------------------------------------------

Thank you for the feedback. Will look into it.

This thread has been idle, in the needs work state with no activity for several months. Therefore, I am assuming that you are no longer pursuing this application, and I marked it as Closed (won't fix).

If this is incorrect, and you are still pursuing this application, then please feel free to re-open it and set the issue status to Needs work or Needs review, depending on the current status of your code.