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

[Upload|Browser] Helpers are PHPCR-ODM specific #66

Closed dantleech closed 11 years ago

dantleech commented 11 years ago

We should probably be putting these classes, currently in File/ in Doctrine\Phpcr\File or similar and naming the services appropriately.

rmsint commented 11 years ago

How are they PHPCR-ODM specific? As far as I see all persistance layer specific things are injected and only need to be changed using a different configuration. The config for phpcr is done in persistance-phpcr.xml, for orm it can be done in persistance-orm.xml, or not?

dantleech commented 11 years ago

Up, yeah I can see now that the classes are only Doctrine specific (will we ever have Propel or "X" support?).

The reason I raised the ticket was because I saw that the cmf_media.upload_file_helper service is phpcr specific, why is the manager class called cmf_media.persistence.phpcr.manager but the upload file helper is only called cmf_media.upload_file_helper?

rmsint commented 11 years ago

Ah I see, good point about Propel or "X" support. I guess we then need to update several bundles as we use the Doctrine specific logic also in other bundles the same way.

The reason to name it cmf_media.upload_file_helper and not cmf_media.persistance.phpcr.upload_file_helper is that we then could use the service regardless of needing to know what persistence type it is. What we know is that there always is one cmf_media.upload_file_helper. We use it also this way in the CreateBundle. We also could name it cmf_media.persistance.phpcr.upload_file_helper and add an alias cmf_media.upload_file_helper to achieve the same.

dbu commented 11 years ago

+1 about the service + alias thing. if the helper class is doctrine specific, we could put the code into the Doctrine namespace but not Doctrine\Phpcr. or call it DoctrineUploadFileHelper.

@rmsint can you do that by monday?

rmsint commented 11 years ago

ok, will do