Problem/Motivation

Some hooks were missed in smart_trim.tokens.inc and need to be converted / moved to .module.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork smart_trim-3593272

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

nicxvan created an issue. See original summary.

nicxvan’s picture

nicxvan commented
Version: 2.0.x-dev » 2.x-dev

nicxvan changed the visibility of the branch 3593272-handle-token-hook to hidden.

nicxvan opened merge request !128

nicxvan’s picture

nicxvan commented
Status: Active » Needs review

Ok I handled converting the token hooks and the hook_hook_info include deprecation.
As part of that I removed the custom token service since it can now just be the hooks.

I then cleaned up phpcs and phpstan issues.

lostcarpark’s picture

lostcarpark commented
Status: Needs review » Reviewed & tested by the community

I have reviewed this change, and It all looks good to me.

I'm very happy to see the back of the tokens.inc file!

Converting the SmartTrimTokens service to SmartTrimTokenHooks.php seems clean and elegant.

I carried out a manual test and everything worked smoothly (once I remembered to rerun ddev symlink-project to get rid of the symlinked tokens.inc file).

Moving to RTBC.

ultimike’s picture

ultimike
Primary language English
Location Florida, USA
commented
Status: Reviewed & tested by the community » Fixed

Looks good - solid refactoring. Appreciate your time, @nicxvan!

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.

  • ultimike committed 4c60a777 on 2.x authored by nicxvan 
    task: #3593272 Handle token hook implementations

By: nicxvan
By:...
ultimike’s picture

ultimike
Primary language English
Location Florida, USA
commented
Status: Fixed » Needs work

I merged, but maybe I shouldn't have just yet - I ran all tests (including prev and next major) and there are some failures...

See https://git.drupalcode.org/project/smart_trim/-/pipelines/837109

-mike

nicxvan’s picture

nicxvan commented

I think these should be set to allow failure since they are so future looking.

phpstan max php version is failing due to:

Call to deprecated method setAccessible() of class ReflectionMethod.  
         🪪  method.deprecated

That is just a deprecation, and it's only deprecated in the maximum php version allowed. This doesn't have to technically be resolved until you want to support php 9.

phpstan next major is failing due to:
the info.yml not having 12 as a core version, this module doens't support 12 yet, so there is nothing to be done here.

phpstan previous major is failing due to:
No error to ignore is reported on line 375.
There are some phpstan ignores added here because accessing dynamic properties is deprecated. However in the previous version php had not yet deprecated it yet so there are no errors.