Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Hey, found some best practice issues that I'll try to fix :), here's the list
vagrant@homestead:~/Code/d8/modules/contrib/google_analytics$ phpcs --standard=DrupalPractice --extensions=php,module,inc,install,test,profile,theme,info,txt,md .
FILE: ...Code/d8/modules/contrib/google_analytics/google_analytics.module
----------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
----------------------------------------------------------------------
10 | WARNING | @author tags are not usually used in Drupal, because
| | over time multiple contributors will touch the code
| | anyway
28 | WARNING | Global constants should not be used, move it to a
| | class or interface
----------------------------------------------------------------------
FILE: ...b/google_analytics/src/Form/GoogleAnalyticsAdminSettingsForm.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 37 WARNINGS AFFECTING 31 LINES
----------------------------------------------------------------------
136 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
153 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
154 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
156 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
158 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
159 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
160 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
161 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
164 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
195 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
196 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
221 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
222 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
223 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
274 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
274 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
274 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
274 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
282 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
324 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
324 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
324 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
324 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
332 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
401 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
410 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
457 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
466 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
488 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
578 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
584 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
597 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
606 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
615 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
618 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
621 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
624 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
----------------------------------------------------------------------
FILE: .../modules/google_analytics_test/google_analytics_test.routing.yml
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
7 | WARNING | Open page callback found, please add a comment before
| | the line why there is no access restriction
----------------------------------------------------------------------
FILE: ..._analytics_test/src/Controller/GoogleAnalyticsTestController.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 4 WARNINGS AFFECTING 4 LINES
----------------------------------------------------------------------
20 | WARNING | Messages are user facing text and must run through
| | t() for translation
21 | WARNING | Messages are user facing text and must run through
| | t() for translation
22 | WARNING | Messages are user facing text and must run through
| | t() for translation
23 | WARNING | Messages are user facing text and must run through
| | t() for translation
----------------------------------------------------------------------
Time: 365ms; Memory: 12Mb
Comment | File | Size | Author |
---|---|---|---|
#4 | corrected_patch_for_t-2934105-1.patch | 1.19 KB | prafullsranjan |
| |||
#2 | google_analytics-drupal_practices_t_and_dependecy_injection-2934105-2-8.patch | 21.62 KB | teeyo |
Comments
Comment #2
teeyo CreditAttribution: teeyo as a volunteer commentedComment #4
prafullsranjan CreditAttribution: prafullsranjan at OpenSense Labs for DrupalFit commentedHere's the correct patch for t();
Comment #8
hass CreditAttribution: hass commented