[PoC] Add writing interfaces #20
Conversation
| @@ -138,6 +141,10 @@ public function write($key, $content) | |||
| $this->setFileDefaults($filePath, $file, $parent); | |||
| } | |||
|
|
|||
| if (! $file instanceof FileWriteInterface) { | |||
| return; | |||
| } | |||
There was a problem hiding this comment.
what is gaufrette expecting here? should this not throw an exception?
There was a problem hiding this comment.
I checked the interface and it expected a boolean as return. Maybe when we write tests and want to use a write interface we can check if an exception is more appropriate.
There was a problem hiding this comment.
if it expects boolean you should return false i think
There was a problem hiding this comment.
huh, I thought it already did. It's corrected thx!
|
after long discussions, we settled in other places to this agreement: http://symfony.com/doc/master/cmf/contributing/bundles.html#interface-naming the gist is: XyInterface is for read and write. if we see a use case for read only, there can be an XyReadInterface and the XyInterface must extend the XyReadInterface. if there is a use case for write only, it would be an XyWriteInterface. unless i am mistaken, this leaves us the path open to add specific read or write only interfaces later, as needed. do you think you can adjust this PR to that philosophy? i notice there are quite some other smaller and bigger cleanups in this PR that should be ported to the current code even if we do not touch the interfaces for now. |
|
I think some fixes from here already in the master branch and some others are ported in the Create PR. I think we do not want to use split read/write interfaces for the MediaBundle atm, shall we close this PR? |
|
agreed. ported the other cleanups in #35 |
A proof-of-concept to see how it would look like for the MediaBundle. We have had a discussion about separating setters and getters on the weekly irc-meeting in general for other bundles.
These related PRs need to be updated if the writing interfaces will be used: