After running coder for Drupal best practices, I found the following issues:

FILE: ...al8/web/modules/contrib/web_accessibility/src/Form/AdminForm.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 55 | WARNING | t() calls should be avoided in classes, use
    |         | dependency injection and $this->t() instead
----------------------------------------------------------------------


FILE: ...8/web/modules/contrib/web_accessibility/web_accessibility.module
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 30 | WARNING | There must be no blank line following an inline
    |         | comment
----------------------------------------------------------------------

Time: 34ms; Memory: 4Mb
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rteijeiro created an issue. See original summary.

  • rteijeiro authored f24f514 on 8.x-1.x
    Issue #2947918 by rteijeiro, cmd87: Fix Drupal best practices
    
cmd87’s picture

I've fixed Drupal BestPractices issues for files listed in description, but there is still some issues with this on README.md file, see here https://pareview.sh/pareview/https-git.drupal.org-project-web_accessibil...

@rteijeiro Please provide a patch to fix this.

cmd87’s picture

Assigned: Unassigned » rteijeiro
rteijeiro’s picture

Assigned: rteijeiro » Unassigned
Status: Active » Needs work
Issue tags: +Novice, +Coding standards

Thanks for the review @cmd87!

Unassigning to let someone else to provide the patch. It's a good issue for a novice contributor ;)

pvsureshmca’s picture

Assigned: Unassigned » pvsureshmca
pvsureshmca’s picture

I have found below list of coding standard issue in the last run.

Review of the 8.x-1.x branch (commit 312a7f4):
Your README.md does not follow best practices (headings need to be uppercase). See https://www.drupal.org/node/2181737 .
The INTRODUCTION section is missing.
The REQUIREMENTS section is missing.
The INSTALLATION section is missing.
The CONFIGURATION section is missing.
Coder Sniffer has found some issues with your code (please check the Drupal coding standards).

FILE: .../web/vendor/drupal/pareviewsh/pareview_temp/web_accessibility.module
--------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
--------------------------------------------------------------------------
 30 | ERROR | [x] Concat operator must be surrounded by a single space
 30 | ERROR | [x] Concat operator must be surrounded by a single space
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

Time: 490ms; Memory: 4Mb
pvsureshmca’s picture

Assigned: pvsureshmca » Unassigned
Status: Needs work » Needs review
FileSize
3.76 KB

Hi,

Please verify the attached patch for above listed best practice issue.

Thank you.

ankitjain28may’s picture

  1. +++ b/web_accessibility.module
    @@ -27,7 +27,7 @@ function web_accessibility_form_node_form_alter(&$form, FormStateInterface $form
    +    '#markup' => '<label>' . t('Check with').'</label>',
    

    There is one more concat operator in this file which need to be fixed too.

ankitjain28may’s picture

Interdiff and patch for the above issue.

pvsureshmca’s picture

Status: Needs review » Reviewed & tested by the community

Hi,

The above patch seems to be working fine for me.

  • ankitjain28may authored 93a30b6 on 8.x-1.x
    Issue #2947918 by ankitjain28may, pvsureshmca, rteijeiro, cmd87: Fix...
cmd87’s picture

Commited and pushed to 8.x-1.x.

Thanks!

cmd87’s picture

Status: Reviewed & tested by the community » Fixed
cmd87’s picture

Status: Fixed » Closed (fixed)