Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove support for PHP 8.0, Remove hard dependency on service manager #76

Merged

Conversation

gsteel
Copy link
Member

@gsteel gsteel commented Jul 20, 2023

Further to discussion in #75, removing the hard-dependency on laminas/service-manager here allows consumers to use the component in projects that require psr/container:2, providing they only use the StandaloneExtensionManager for Reader\ and Writer\

…ict consumers to psr/container:1.0

Signed-off-by: George Steel <george@net-glue.co.uk>
Signed-off-by: George Steel <george@net-glue.co.uk>
@gsteel
Copy link
Member Author

gsteel commented Jul 20, 2023

Ughh - we have to drop PHP 8.0 and work around the BC break in tests introduced in SM 3.21

…inimum and fix pluginmanager compatibility tests

Signed-off-by: George Steel <george@net-glue.co.uk>
Signed-off-by: George Steel <george@net-glue.co.uk>
Signed-off-by: George Steel <george@net-glue.co.uk>
Signed-off-by: George Steel <george@net-glue.co.uk>
@froschdesign
Copy link
Member

@gsteel
Thanks for the quick solution! 👍🏻

@MegaChriz
Can you check this?

@gsteel gsteel changed the title Remove hard dependency on service manager Remove support for PHP 8.0, Remove hard dependency on service manager Jul 20, 2023
@MegaChriz
Copy link

Thanks for picking this up so quickly! On my local install I have PHP 8.1 and that looked to go well. I saw that on a composer update on a Drupal 10 install that laminas/laminas-servicemanager got removed. Unit and Kernel tests for Feeds still passed. The Functional tests I did not run yet, as they take more time to run on my local machine.

I hope to test with PHP 8.2 soon!

@MegaChriz
Copy link

Feeds does indeed use \Laminas\Feed\Reader\StandaloneExtensionManager.

These are other classes/interfaces that Feeds uses:

  • Laminas\Feed\Reader\Exception\ExceptionInterface
  • Laminas\Feed\Reader\ExtensionManagerInterface
  • Laminas\Feed\Reader\Reader
  • Laminas\Feed\Reader\Extension\AbstractEntry
  • Laminas\Feed\Reader\FeedSet

@gsteel
Copy link
Member Author

gsteel commented Jul 21, 2023

This just needs an approving review from someone on the @laminas/technical-steering-committee and it can be released

@MegaChriz
Copy link

Feeds tests pass
I ran all Feeds tests locally now on PHP 8.2.8 and all that are at least related to parsing RSS Feeds passed!

Feeds can now be installed using composer
I also tested if I could install drupal/feeds on PHP 8.2.8 now by using the following definitions in composer.json of a local project:

"repositories": [
   (...)
   {
        "type": "vcs",
        "url": "https://github.com/gsteel/laminas-feed.git"
    }
],
"require": {
   (...)
    "laminas/laminas-feed": "dev-remove-hard-dependency-on-service-manager as 2.13"
},

With that I could require drupal/feeds with success!
(And without the changes from this issue composer reports that the requirements could not be resolved.)

Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @gsteel!

@Ocramius Ocramius self-assigned this Jul 24, 2023
@Ocramius Ocramius merged commit 32076e2 into laminas:2.21.x Jul 24, 2023
9 checks passed
@Ocramius Ocramius added this to the 2.21.0 milestone Jul 24, 2023
@gsteel gsteel deleted the remove-hard-dependency-on-service-manager branch July 24, 2023 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants