Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Change static interface to dynamic #19

Open
necromant2005 opened this issue Jan 17, 2016 · 6 comments
Open

Change static interface to dynamic #19

necromant2005 opened this issue Jan 17, 2016 · 6 comments

Comments

@necromant2005
Copy link

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.

@weierophinney
Copy link
Member

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!

@necromant2005
Copy link
Author

Thank you Matthew! I'll start migration.

@necromant2005
Copy link
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,

@necromant2005
Copy link
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;

@necromant2005
Copy link
Author

I've also migrated cache interface to PSR-6

@weierophinney
Copy link
Member

This repository has been closed and moved to laminas/laminas-feed; a new issue has been opened at laminas/laminas-feed#12.

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

No branches or pull requests

3 participants