Problem/Motivation

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

Steps to reproduce

Look into the module's source code, it is using outdated PHP practices.

Proposed resolution

Upgrade the module follow recent standard practices for writing more robust PHP code.

Remaining tasks

N/A

User interface changes

N/A

API changes

N/A

Data model changes

N/A

CommentFileSizeAuthor
#8 3581194-better-php-standard.patch0 bytesyusuf_khan

Issue fork timepicker-3581194

Command icon Show commands

Start 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:

About issue forks

Comments

danrod created an issue. See original summary.

danrod’s picture

danrod
Location Ottawa
commented
Issue tags: +Novice
jatin.buzz’s picture

jatin.buzz commented

Working on It!

jatin.buzz opened merge request !39

jatin.buzz’s picture

jatin.buzz commented
Status: Active » Needs review

Please review the MR!
These are some changes I have made
Added parameter and return type hints to hook wrappers and hook service methods.
Replace loose comparisons with strict comparisons where behavior is unchanged.
Update hook example return type in API documentation.

yusuf_khan’s picture

yusuf_khan commented
Status: Needs review » Needs work

I've reviewed the Timepicker module and found the following PHPCS and PHP stan issues that need to be fixed:

vendor/bin/phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,info,txt,md,yml web/modules/contrib/timepicker-3581194/

FILE: /Users/ykhan/Documents/my-drupal-site/web/modules/contrib/timepicker-3581194/README.md
--------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
--------------------------------------------------------------------------------------------
70 | WARNING | Line exceeds 80 characters; contains 183 characters
80 | WARNING | Line exceeds 80 characters; contains 119 characters
83 | WARNING | Line exceeds 80 characters; contains 123 characters
--------------------------------------------------------------------------------------------

FILE: /Users/ykhan/Documents/my-drupal-site/web/modules/contrib/timepicker-3581194/timepicker.install
-----------------------------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
-----------------------------------------------------------------------------------------------------
12 | ERROR | [x] Opening brace should be on the same line as the declaration
53 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
54 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
63 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
-----------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------

FILE: /Users/ykhan/Documents/my-drupal-site/web/modules/contrib/timepicker-3581194/timepicker.module
----------------------------------------------------------------------------------------------------
FOUND 7 ERRORS AFFECTING 7 LINES
----------------------------------------------------------------------------------------------------
22 | ERROR | [x] Opening brace should be on the same line as the declaration
33 | ERROR | [x] Opening brace should be on the same line as the declaration
46 | ERROR | [x] Opening brace should be on the same line as the declaration
57 | ERROR | [x] Opening brace should be on the same line as the declaration
73 | ERROR | [x] Opening brace should be on the same line as the declaration
84 | ERROR | [x] Opening brace should be on the same line as the declaration
115 | ERROR | [x] Opening brace should be on the same line as the declaration
----------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 7 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------

Time: 191ms; Memory: 14MB

vendor/bin/phpstan analyze web/modules/contrib/timepicker-3581194 --memory-limit=1G
4/4 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

------ ------------------------------------------------------------------------------------------------------------------
Line src/Hook/TimepickerHooks.php
------ ------------------------------------------------------------------------------------------------------------------
:39 No error to ignore is reported on line 39.
🪪 ignore.unmatchedLine (non-ignorable)
:41 Method Drupal\timepicker\Hook\TimepickerHooks::help() should return string|null but return statement is missing.
🪪 return.missing (non-ignorable)
:90 No error to ignore is reported on line 90.
🪪 ignore.unmatchedLine (non-ignorable)
:165 No error to ignore is reported on line 165.
🪪 ignore.unmatchedLine (non-ignorable)
:220 No error to ignore is reported on line 220.
🪪 ignore.unmatchedLine (non-ignorable)
:326 No error to ignore is reported on line 326.
🪪 ignore.unmatchedLine (non-ignorable)
------ ------------------------------------------------------------------------------------------------------------------

------ ---------------------------------------------
Line timepicker.module
------ ---------------------------------------------
:31 No error to ignore is reported on line 31.
🪪 ignore.unmatchedLine (non-ignorable)
:55 No error to ignore is reported on line 55.
🪪 ignore.unmatchedLine (non-ignorable)
:82 No error to ignore is reported on line 82.
🪪 ignore.unmatchedLine (non-ignorable)
:95 No error to ignore is reported on line 95.
🪪 ignore.unmatchedLine (non-ignorable)
:113 No error to ignore is reported on line 113.
🪪 ignore.unmatchedLine (non-ignorable)
------ ---------------------------------------------

[ERROR] Found 11 errors

danrod’s picture

danrod
Location Ottawa
commented
Assigned: Unassigned » danrod
yusuf_khan’s picture

yusuf_khan commented
StatusFileSize
new 3581194-better-php-standard.patch0 bytes

@danrod , Please check the attached Patch can fix the pipeline issues - PHPCS and PHPSTAN

Im not able to push or create a branch in the repo, please check the error below
You are not allowed to push code to this project.

yusuf_khan’s picture

yusuf_khan commented
Status: Needs work » Needs review
danrod’s picture

danrod
Location Ottawa
commented

Hi @yusuf_khan , if you click on the "Get push access" button above, you'll be able to push code to this project. We are not using patches for our projects anymore.

danrod’s picture

danrod
Location Ottawa
commented
Status: Needs review » Needs work

jerech made their first commit to this issue’s fork.

jerech’s picture

jerech commented
Status: Needs work » Needs review
danrod’s picture

danrod
Location Ottawa
commented

I tested this and works as expected (I was able to pick the JS library and use it when creating/editing entities). I'm merging this, thanks a lot to everyone who worked on this !

danrod’s picture

danrod
Location Ottawa
commented
Status: Needs review » Reviewed & tested by the community

  • danrod committed 7ee565d3 on 3.0.x authored by jatin.buzz 
    Issue #3581194: Improve PHP standards and type declarations.
danrod’s picture

danrod
Location Ottawa
commented
Status: Reviewed & tested by the community » Fixed

Merged with no issues, I'll create a new release.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

danrod’s picture

danrod
Location Ottawa
commented

New release: https://www.drupal.org/project/timepicker/releases/3.0.6