Skip to content

Conversation

Nyholm
Copy link
Member

@Nyholm Nyholm commented Aug 9, 2016

This will fix the issue with the RecorderCachePool (used for Symfony data collection) implements TaggableCache. If the underlying cache implemnation (like apc cache) does not support tagging we have to decorate the implementation with TaggablePSR6PoolAdapter. But that causes trouble when using a ChainPool.

This PR introduce a Factory that choose a RecordingPool that implements the same interfaces as the CachePool.

@codecov-io
Copy link

codecov-io commented Aug 9, 2016

Current coverage is 23.57% (diff: 1.19%)

Merging #65 into master will decrease coverage by 1.13%

@@             master        #65   diff @@
==========================================
  Files            19         21     +2   
  Lines           781        806    +25   
  Methods          87         89     +2   
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits            193        190     -3   
- Misses          588        616    +28   
  Partials          0          0          

Powered by Codecov. Last update 0edd96a...0eedb1d

@Nyholm
Copy link
Member Author

Nyholm commented Aug 10, 2016

@php-cache/featured-users What do you think?


// Tell the collector to add the new logger
$collectorDefinition->addMethodCall('addInstance', [$id, new Reference($id.'.recorder')]);
$collectorDefinition->addMethodCall('addInstance', [$id, new Reference($id)]);
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe update this comment

@Nyholm
Copy link
Member Author

Nyholm commented Aug 15, 2016

I'll add a test for the compiler pass and then I'll merge this PR.

@Nyholm Nyholm merged commit 7766048 into master Aug 15, 2016
@Nyholm Nyholm deleted the recording-pool-fix branch August 15, 2016 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants