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

Change static interface to dynamic #12

Open
weierophinney opened this issue Dec 31, 2019 · 5 comments
Open

Change static interface to dynamic #12

weierophinney opened this issue Dec 31, 2019 · 5 comments

Comments

@weierophinney
Copy link
Member

I know it may be a bit too complicated due as part of migration to zf3. It makes sense to change static zend-feed interface to dynamic.

class Reader 
{
    public function __construct(HttpClient $http = null, CacheAdapter $cache = null)

    public function import()
    {
        $cache   = $this->getCache();
        $client  = $this->getHttpClient();
        $cacheId = 'Zend_Feed_Reader_' . md5($uri);
        ...
   }
}

It will break back comparability with version 2.*. But It's necessary changes if we want to have clear, easy management code that can be used in any application without global static variables.


Originally posted by @necromant2005 at zendframework/zend-feed#19

@weierophinney
Copy link
Member Author

Having done the work to separate the plugin manager and http dependencies, I can attest to the difficulties in testing the compliment currently; static usage makes this quite cumbersome!

If we're going to break BC anyway, I'd argue we should also target PSR-6 interfaces (particularly since with is being done to provide psr-6 compatibility in zend-cache). That particular change would affect also the pubsdubhubbub subcomponent, but not the writer.

to;dr: go for it!


Originally posted by @weierophinney at zendframework/zend-feed#19 (comment)

@weierophinney
Copy link
Member Author

Thank you Matthew! I'll start migration.


Originally posted by @necromant2005 at zendframework/zend-feed#19 (comment)

@weierophinney
Copy link
Member Author

Hi Matthew,

I've migrated Zend-Feed reader https://github.com/necromant2005/zend-feed/tree/BUG19-mirgration-static-tdynamic
tests passed: https://travis-ci.org/necromant2005/zend-feed/builds/110578421

I can merge all changes in one commit, if you prefer.

Cheers,


Originally posted by @necromant2005 at zendframework/zend-feed#19 (comment)

@weierophinney
Copy link
Member Author

Also it will be great to migrate "writter" because it uses the same weird static plugin manager functionality
/**
* @var ExtensionManagerInterface
*/
protected static $extensionManager = null;


Originally posted by @necromant2005 at zendframework/zend-feed#19 (comment)

@weierophinney
Copy link
Member Author

I've also migrated cache interface to PSR-6


Originally posted by @necromant2005 at zendframework/zend-feed#19 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants