Problem/Motivation

FILE: /var/www/pareviewd/pareview_temp/lwdalmnb/cloudflare_stream.module
------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------
1 | ERROR | [x] Missing file doc comment
------------------------------------------------------------------------


FILE: ...re_stream_hosted_video/src/Plugin/media/Source/HostedVideoSource.php
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
21 | ERROR | [x] There must be exactly one newline after the class
| | comment
--------------------------------------------------------------------------


FILE: ...flare_stream_hosted_video/config/install/media.type.hosted_video.yml
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
14 | ERROR | [x] Expected 1 newline at end of file; 2 found
--------------------------------------------------------------------------


FILE: ...ideo/config/install/field.storage.media.field_media_hosted_video.yml
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
23 | ERROR | [x] Expected 1 newline at end of file; 2 found
--------------------------------------------------------------------------


FILE: .../install/field.field.media.hosted_video.field_media_hosted_video.yml
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
26 | ERROR | [x] Expected 1 newline at end of file; 2 found
--------------------------------------------------------------------------
CommentFileSizeAuthor
#7 3236369-7.patch2.51 KBmohit.bansal623
#2 3236369-2.patch3.66 KBmohit.bansal623

Comments

pjbaert created an issue. See original summary.

mohit.bansal623’s picture

mohit.bansal623 commented
Status: Active » Needs review
StatusFileSize
new 3236369-2.patch3.66 KB

Please review this patch.

pjbaert’s picture

pjbaert
Primary language Dutch
commented
Status: Needs review » Needs work

Thanks for your work.

Let's fix these comments:

  1. +++ b/cloudflare_stream.module
@@ -1,5 +1,25 @@
+ * This file will contain hooks and basic functions.
...
+      $output .= '<p>' . t('This modules integrates Cloudflare Stream as a new field which lets you upload video in almost any format and stream high quality video on any device without having to build your own streaming solution from scratch.') . '</p>';

    I prefer a more describing sentence.
    Let's update this comment to a description of what this module is doing. "this module integrates Cloudflare Stream ..." 

  2. +++ b/cloudflare_stream.module
@@ -1,5 +1,25 @@
+/**
+ * Implements hook_help().
+ */
+function cloudflare_stream_help($route_name, RouteMatchInterface $route_match) {
+  switch ($route_name) {
+    case 'help.page.cloudflare_stream':
+      $output = '';
+      $output .= '<h3>' . t('About') . '</h3>';
+      $output .= '<p>' . t('This modules integrates Cloudflare Stream as a new field which lets you upload video in almost any format and stream high quality video on any device without having to build your own streaming solution from scratch.') . '</p>';
+      return $output;
+  }
+}

    Let's fix this in #3236358: Implement hook_help() 

  3. +++ b/modules/cloudflare_stream_hosted_video/src/Plugin/media/Source/HostedVideoSource.php
@@ -34,10 +33,12 @@ class HostedVideoSource extends File {
+    $display->setComponent(
+          $this->getSourceFieldDefinition($type)->getName(), [
+            'type' => 'cloudflarevideo_default',
+            'label' => 'visually_hidden',
+          ]
+      );

    I believe the current code is ok by Drupal coding standards. Let's revert this.

mohit.bansal623’s picture

mohit.bansal623 commented
Status: Needs work » Needs review

No need to merge the above patch.
hook_help() is in another ticket.
What changes are needed in description, I took this from module page only and seems meaningful also.

tim-diels’s picture

tim-diels
He/Him
Primary language Dutch
Location Belgium 🇧🇪
commented
Status: Needs review » Needs work

@mohit.bansal623 please use the status correctly. If there is a discussion need to be done, this is not in need review.

For me the patch is not correct, as @pjbaert also stated.

Please remove the hook_help and fix the coding standards as needed before it can be back in need review.

mohit.bansal623’s picture

mohit.bansal623 commented

@tim-diels - Noted. I'll take care of this. Again recreating the patch for the changes which needs to be done.

mohit.bansal623’s picture

mohit.bansal623 commented
Status: Needs work » Needs review
StatusFileSize
new 3236369-7.patch2.51 KB

Now I have only done the changes specific to this ticket and created the patch. Please review.

  • pjbaert committed d4353e8 on 8.x-2.x 
    Issue #3236369 by mohit.bansal623, pjbaert, tim-diels: Fix codesniffer...
pjbaert’s picture

pjbaert
Primary language Dutch
commented
Status: Needs review » Fixed 
+++ b/cloudflare_stream.module
@@ -1,5 +1,10 @@
+ * This file will contain hooks and basic functions.

As a small personal remark, I updated this @file doc comment from the patch to Hook implementations for Cloudflare Stream module.

Thanks for fixing these small codesniffer nitpicks & thank you for reviewing. Pushed to 2.x branch!

Status: Fixed » Closed (fixed)

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