Move the hook implementations to OOP hooks.

Issue fork scanner-3586669

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

damienmckenna created an issue. See original summary.

grask0’s picture

grask0 commented
Assigned: Unassigned » grask0

grask0 opened merge request !83

grask0’s picture

grask0 commented
Status: Active » Needs review
damienmckenna’s picture

damienmckenna
Location TN, USA
commented

Thank you for putting that together. I've launched the tests against D10 to confirm there aren't any regressions.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Assigned: grask0 » Unassigned
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs review » Needs work

The test run against D10 fails, so this needs some additional work.

grask0’s picture

grask0 commented
Status: Needs work » Needs review

Sorry, my bad. Now it should be ok.

Thanks!

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs review » Needs work

Thank you for that.

Would you mind using the other approach for the backwards compatibility - add items to the services.yml file for each hook and then call the the services in the old hook code? That should be cleaner for D10 sites, and avoids needing to add the phpstan changes to make the testbot ignore the problems. Thank you again.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

This needs to be updated after some recent changes to the 2.0.x branch.

grask0’s picture

grask0 commented
Status: Needs work » Needs review

Hey, sorry for the delay!

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs review » Reviewed & tested by the community

Thank you!

smustgrave’s picture

smustgrave commented
Status: Reviewed & tested by the community » Needs work

My mistake I went ahead and merged the bot one https://www.drupal.org/project/scanner/issues/3602660

So this one maybe we can just cleanup what it did and better organize?

smustgrave closed merge request !83

smustgrave opened merge request !106

  • smustgrave committed 85aa4da2 on 2.0.x 
    task: #3586669 Replace hook implementations with OOP hooks
smustgrave’s picture

smustgrave commented
Status: Needs work » Fixed

Rebased and cleaned up some. Thanks.

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.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.