I see that the module follows very old PHP standard practices, for example it doesn't follow the function return type hinting, among other features that need to be added
More information about Drupal's coding standards
Look into the module's source code, it is using outdated PHP practices.
Upgrade the module follow recent standard practices for writing more robust PHP code.
N/A
N/A
N/A
N/A
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes, plain diff MR !16changes, plain diff MR !15
Comments
Comment #2
danrodComment #4
sapnil_biswas commentedI would like to work on this
Comment #6
sapnil_biswas commentedmarking this as needs review please let me know for any further changes required
Comment #7
vinodhini.e commentedHi,
Tested this on Drupal 10.5.1 and ran coding standards using PHPCS, ESLint, and Stylelint.
After applying MR !15 and re-running the reports:
ESLint and Stylelint are not reporting any issues.
PHPCS is showing warnings only (no errors), listed below.
FILE: /var/www/html/web/modules/contrib/jqcloud-3581168/tests/src/FunctionalJavascript/JqcloudJavascriptTest.php
FOUND 0 ERRORS AND 19 WARNINGS AFFECTING 19 LINES
All warnings are related to the usage of t() in classes, suggesting the use of StringTranslationTrait and $this->t() instead.
Thanks.
Comment #8
sapnil_biswas commentedThanks @vinodhini.e , for testing this out I have tried to resolve the issues in 25c9ead8
Comment #9
vinodhini.e commentedHi, @sapnil_biswas,
I applied your changes and re-ran the PHPCS report. No errors or warnings are being reported, and its working fine.
Thanks.
Comment #10
sapnil_biswas commentedComment #11
sapnil_biswas commented@danrod Could you please review this once when you have time and let me know if there is anything i am currently missing at
Comment #12
danrodHi @sapnil_biswas , Could you create a MR against the 2.0.x branch?
Comment #13
danrodComment #17
sapnil_biswas commentedMR 16 is based on 2.0.x. @danrod please review it when you have time
Comment #19
danrodSorry @sapnil_biswas my mistake !!!
I meant branch 3.0.x, I got confused with another project.
I'm setting this to "Needs Work" again, I can do this myself if don't feel like doing it again.
Comment #20
danrodComment #21
sapnil_biswas commentedThats completely fine. I will get this done asap
Comment #22
sapnil_biswas commentedupdated the changes on 3.0.x , marking as needs review
Comment #24
jerech commentedI've updated the MR to use the PHPUnit annotation approach for cross-version compatibility.
Verified locally and in GitLab CI:
* PHPCS ✅
* PHPStan ✅
* PHPUnit ✅
* Stylelint ✅
* CSpell ✅
All pipelines are now passing and the issue is ready for review/merge.
Comment #25
danrodThank you all, I also tested functionality and works as expected, merging this.
Thanks again !
Comment #27
danrod