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

YAML mappings #37

Merged
merged 6 commits into from
Nov 30, 2015
Merged

YAML mappings #37

merged 6 commits into from
Nov 30, 2015

Conversation

jordonsc
Copy link
Member

@y2khjh
Copy link

y2khjh commented Nov 30, 2015

What is the default behavior of __toString() for these YAML generated entity? And how to change it?

@jordonsc
Copy link
Member Author

An entity and a mapping are different things, this is confusing given that currently mappings are only supported via annotations - which is the mapping data & entity class in one.

Now you would have normal classes with no annotations, and a separate YAML file that explains what each property and relationship, etc is.

@y2khjh
Copy link

y2khjh commented Nov 30, 2015

OK

* @param string $class
* @return string
*/
protected function getTableForClass($class)
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, not what it's intended for (will discuss that with you in Slack).

That function is a helper for the managers sub-services - it just pulls entity metadata and gets the table name - you want the opposite. This problem though (which is the refs problem) is what this PR is heading down the direction of fixing. Although I'm still not sure how to support annotation mappings with the ref problem fixed (Yaml mappings can do it safely).

$data[Schema::COLUMN_ID] = true;
}

$default_getter = 'get'.Inflector::classify($column->getProperty());
Copy link
Member

Choose a reason for hiding this comment

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

http://symfony.com/doc/current/components/property_access/introduction.html ? may be there are other places that can benefit from this? just a thought.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah Tony was telling me about that at lunch - I think changing the inflector might count as another PR, though.

Copy link
Member

Choose a reason for hiding this comment

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

cool

@purinda
Copy link
Member

purinda commented Nov 30, 2015

👍

jordonsc added a commit that referenced this pull request Nov 30, 2015
@jordonsc jordonsc merged commit 57ac19e into master Nov 30, 2015
@jordonsc jordonsc deleted the feature/yaml-mapper branch November 30, 2015 06:16
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.

5 participants