Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] model + adapters #5

Merged
merged 7 commits into from
Jul 4, 2013
Merged

[WIP] model + adapters #5

merged 7 commits into from
Jul 4, 2013

Conversation

// perhaps add a DirectoryInterface extending FileInterface?
// with the methods isDirectory and getDirectory,
// getDirectory returns the parent
// see also directory methods Local adapter
Copy link
Member

Choose a reason for hiding this comment

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

in phpcr there is nt:folder which we also have a document class for in phpcr-odm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes exactly, for PHPCR we want to use the nt:folder as "directory". I think we need to add something like ``directoryClass` to the constructor aswell. Or when using FileProviders, it can be abstracted there.

@dbu
Copy link
Member

dbu commented Jun 17, 2013

mixins are PHPCR level constructs. the primary node type tells what properties a node may have. it can also allow everything, as nt:unstructured does. but nt:file for example defines exactly what fields a node can have. mixins can be added to a node to tell it may have or must have additional properties. to see an example mixin, see here: https://github.com/doctrine/phpcr-odm/blob/master/lib/Doctrine/ODM/PHPCR/Tools/Console/Command/RegisterSystemNodeTypesCommand.php#L63

there is a mixins xml element in the xml/yml mapping and a mixins property of document for annotation. to provide the new node type, we need to define an initializer service in MenuBundle. wanted to link you the doc and noticed its missing, so i just created it: symfony-cmf/symfony-cmf-docs#163

@rmsint
Copy link
Contributor Author

rmsint commented Jun 17, 2013

ok, cool! The interfaces are ready now imho. This evening I will work on creating PRs and move the PHPCR-ODM Image logic to CmfMedia. I will "try out" the documentation for the mixins and Repository Initializers then :-)

@rmsint
Copy link
Contributor Author

rmsint commented Jun 18, 2013

I think this is now ready for review. I will leave defining the mixins to the hackday. There is one issue, the stream is not saved in the embedded Resource document of the File. That part is copied without modifications from PHPCRODM to CmfMedia. I tried many things to find the cause and cannot find it.

@lsmith77
Copy link
Member

you should rebase on master and update the composer.json dependencies accordingly

@lsmith77
Copy link
Member

also some tests would be nice :)

* determine the identifier)
*/
public function __construct(
ObjectManager $manager,
Copy link
Member

Choose a reason for hiding this comment

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

Should this be ManagerRegistry? And so we should also have setManagerName etc. ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah yes, forget that one, will update.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@dantleech
Copy link
Member

For tests have a look at the MenuBundle and check the open PR on symfony-cmf-docs about implementing the testing component :)

@rmsint
Copy link
Contributor Author

rmsint commented Jun 19, 2013

@lsmith77 rebased

Yes, quickly writing tests is something I have to learn. (@dantleech I am looking forward to learn from the docs.) Depending on when we want to release I would need some help or someone else can pick this up I guess (I don't mind anyways).

@dbu
Copy link
Member

dbu commented Jun 19, 2013

i suggest we solve the obvious and trivial issues and then merge this PR and create new issues so we can fix them individually and add tests. gaufrette is not important for images in sonata admin or replacing the phpcr-odm image for example...
we did not tag this bundle yet so it can be considered experimental. i suggest we start tagging alpha once we feel reasonable confident we don't need any more capital BC breaks.

dbu added a commit that referenced this pull request Jul 4, 2013
[WIP] model + adapters
@dbu dbu merged commit 89e8768 into symfony-cmf:master Jul 4, 2013
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.

4 participants