- Issue created by @cbfannin
- Status changed to Needs review
20 days ago 2:15am 8 June 2024 - πΊπΈUnited States cbfannin
Removed the unnecessary return statement phpcs complained about. This is now passing.
- Status changed to Needs work
19 days ago 1:35am 9 June 2024 - π΅πPhilippines paraderojether
Hi
I reviewed MR!9, and there are still phpcs issues shown below:
contrib phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig views_dependent_filters FILE: /Users/jetherparadero/Q2_DO/q2_issues/web/modules/contrib/views_dependent_filters/README.txt -------------------------------------------------------------------------------------------------- FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE -------------------------------------------------------------------------------------------------- 3 | WARNING | Line exceeds 80 characters; contains 106 characters -------------------------------------------------------------------------------------------------- FILE: ...sers/jetherparadero/Q2_DO/q2_issues/web/modules/contrib/views_dependent_filters/src/Plugin/views/filter/ViewsDependentFilter.php -------------------------------------------------------------------------------------------------------------------------------------- FOUND 0 ERRORS AND 11 WARNINGS AFFECTING 11 LINES -------------------------------------------------------------------------------------------------------------------------------------- 130 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() | | instead 143 | WARNING | Unused variable $form_state_copy. 162 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() | | instead 163 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() | | instead 171 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() | | instead 174 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() | | instead 177 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() | | instead 193 | WARNING | Unused variable $controller_filter. 233 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() | | instead 236 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() | | instead 253 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() | | instead -------------------------------------------------------------------------------------------------------------------------------------- Time: 155ms; Memory: 12MB
- πΊπΈUnited States generalredneck
Odd. CI is running clean. Can you verify paraderojether, you are against the latest commit?
- Status changed to RTBC
18 days ago 2:38pm 10 June 2024 - πΊπΈUnited States generalredneck
@paraderojether,
So, as a heads up, This module is using the default Gitlab CI for Drupal template. You can see it's installation as part of the repository, and the instructions used to install it can be found here: https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr β ....
That said, the phpcs.xml.dist used by the default CI is located here: https://git.drupalcode.org/project/gitlab_templates/-/blob/main/assets/p...
The command you used to check coding standards would not be equivalent to what was checked by CI, but goes above and beyond adding the DrupalPractice standards and additional file types.
While I appreciate any contributions that correct coding standards, Charles has satisfied my criteria for marking this task as complete. I'll leave the issue open for a few more days, as if you wish to receive credit for this issue, you will make the code changes you suggest as a Merge Request. If you wish, you may even add a custom phpcs.xml.dist that holds this module to the higher standards as a value add and ensure that Gitlab CI passes.
Thanks