Updated: Comment #N
Problem/Motivation
The CacheFactory permits overriding of the cache backend service on a per-bin basis from within settings.php. However this is currently broken because CacheFactory::get fails to access the proper key of the settings array.
Proposed resolution
Remaining tasks
None
User interface changes
None
API changes
None
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | 2233337-TEST-ONLY-fix-per-bin-cache-backend-service-14.diff | 3.83 KB | znerol |
| #8 | 2233337-fix-per-bin-cache-backend-service-8.diff | 4.45 KB | znerol |
| #8 | 2233337-TEST-ONLY-fix-per-bin-cache-backend-service-8.diff | 3.83 KB | znerol |
Comments
Comment #1
znerol commentedComment #2
znerol commentedComment #3
Anonymous (not verified) commentedyes, this is awesome.
Comment #4
znerol commentedComment #6
berdirLooks nice, some minor nitpicks...
Contains \Drupal\... ;)
Look at some of the new test classes, they use @coversDefaultClass + @covers on methods, we should do that her as well.
I think most tests use an actual ContainerBuilder instead of mocking it, that's easier I think.
It will also fail nicer (unknown service exception instead of fatal error)
Wondering if we should use something else than contrib here, memory maybe? or example?
Comment #7
berdirComment #8
znerol commentedThanks for the review @Berdir. Changed the name and comment for the test you mentioned in 4, I hope it is now clear what it is about.
Comment #10
berdirStill missing a leading \, but can be fixed on commit...
Looks good, let's fix this.
Comment #11
dawehnerGreat work!
Comment #12
catchCommitted/pushed to 8.x, thanks!
Comment #14
znerol commentedThanks @catch. However
CacheFactoryTestseems to be missing from the commit. Attached the test-only patch, also addressing the remark from #10.Comment #15
catchArggh. 'fixed locally' yeah right...
Committed/pushed to 8.x, thanks!