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

[FEATURE] Added attribute support #63

Open
wants to merge 6 commits into
base: 1.x
Choose a base branch
from

Conversation

seifane
Copy link

@seifane seifane commented Mar 10, 2022

Follwing my issue (#62) I propose this PR as a solution to support attributes.

The reimplementation of Attribute / Annotation reader is needed since MappingDriverChain in orm doesn't give us a reader if it's not an instance of AnnotationDriver which is not the case when using attributes. This method is taken from the doctrine-extensions repo.

Let me know if you think there's a better way of handling this, I'm not sure of myself on this one :).

@dpslwk
Copy link
Member

dpslwk commented Mar 10, 2022

can this be done with out change the php constraint in composer?

the 1.x branch and release of this repo are intended to support multiple laravel version in one code base 6.x, 7.x, 8.x, 9.x which is why the php support is 7 and 8

@seifane
Copy link
Author

seifane commented Mar 10, 2022

@dpslwk I just testing reverting the platform requirements and running it on a 7.4 project and it passed okay. I was just scared about leaving the attributes there for previous versions but it doesn't seem to care.

@eigan
Copy link
Member

eigan commented Mar 12, 2022

I am not using this package so it's really difficult for me to say if this is "the way to go".

Some tests would be great.

@mattzuba
Copy link

mattzuba commented Mar 3, 2024

Adding a two year bump to this - any chance of getting this added in and new release being cut? The ORM package has this and I've used it for all of my entities, but can't use this package now.

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