symfony-cmf / media-bundle

UNMAINTAINED - Minimalistic interfaces to handle media in the context of the CMF
http://cmf.symfony.com/
30 stars 40 forks source link

Fix hard dependency on gd #74

Closed rmsint closed 11 years ago

rmsint commented 11 years ago
Q A
Bug fix? yes
New feature? no
BC breaks? yes
Deprecations? no
Tests pass? yes
Fixed tickets #71
License MIT
Doc PR https://github.com/symfony-cmf/symfony-cmf-docs/issues/302
rmsint commented 11 years ago

I think remaing is now a design question: where and how to put the logic. Just realized that we should keep in mind the integration with sonata to do later. I think with sonata media this Logic would be placed in the image provider. Each media type could have it's own provider to set specific defaults.

dbu commented 11 years ago

it worries me a bit that the user might forget to let the mediamanager update the dimensions. what about offering doctrine listeners (i think one class can suit at least orm and phpcr-odm as its the same event hopefully) for prePersist and preUpdate that call the mediamanager? that way no other code needs to explicitly know about this and it can't be forgotten. i would hope nobody is doing output before flushing doctrine... and if he does, he still can trigger the updating manually.

rmsint commented 11 years ago

The tests should pass now. A listener is indeed a better place yes. (Although I do not really know how to test that ...)

dbu commented 11 years ago

i think this bundle should provide it and load it based on the chosen persistence layer. if you created the listener i think you forgot to add it in git... for the test: you can test the listener itself in a unit test. but a functional test with a running phpcr-odm would be a lot better to see if things work.

rmsint commented 11 years ago

ok, will pick this up this evening again

lsmith77 commented 11 years ago

any progress?

rmsint commented 11 years ago

almost finished and ready for review

rmsint commented 11 years ago

fyi dbu updates the code currently so I won't touch them (talked about it on irc)

dbu commented 11 years ago

okay, so this is now ready i hope. lets see what travis says.