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

[RFC] Parse trait class annotations #1517

Closed
wants to merge 1 commit into from

Conversation

kinncj
Copy link

@kinncj kinncj commented Sep 25, 2015

The parser isn't parsing trait class annotations.
Fixed by doctrine/annotations#63

@doctrinebot
Copy link

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DDC-3916

We use Jira to track the state of pull requests and the versions they got
included in.

@kinncj
Copy link
Author

kinncj commented Sep 28, 2015

ping @guilhermeblanco @FabioBatSilva

@kinncj
Copy link
Author

kinncj commented Oct 2, 2015

@FabioBatSilva, @guilhermeblanco, any updates regarding this PR?

@guilhermeblanco
Copy link
Member

Was supposed to review and merge yesterday, but I forgot.
Will do it during the weekend.

@guilhermeblanco
Copy link
Member

This is more dangerous than beneficial.
I'd suggest (like I've done, since we worked together) to add the annotation at the top of every entity. It's safer.

@kinncj
Copy link
Author

kinncj commented Nov 6, 2015

Some developers are more copy and paster than coders.
Anyway, you are splitting one unique behaviour in two different participants then... the trait with properties and HasLifecycleCallbacks(callbacks) and the annotation for HasLifecycleCallbacks in the main participant (class). which means: the class annotation won't work without the trait and the trait won't do anything if the class doesn't annotate that... so instead of a Trait, it's better to just add the HasLifecycleCallbacks(the callback methods) to the main participant then.

Anyway, we can talk in off.

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.

None yet

3 participants