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

Add doctrine/annotations dependency #711

Merged
merged 1 commit into from May 17, 2017

Conversation

Koc
Copy link
Contributor

@Koc Koc commented May 17, 2017

@garak garak merged commit 853172f into dustin10:master May 17, 2017
@harmstyler
Copy link

Is this necessary as a required dependency? Due to the fact that Annotations are optional for this bundle the annotations library should be optional as well. I'd recommend a composer suggestion over a requirement:

"suggest": {
    "doctrine/annotations": "To use Symfony annotations."
}

@garak
Copy link
Collaborator

garak commented Jun 13, 2017

I agree.
Likely we'll need a line in "require-dev" also

@Koc
Copy link
Contributor Author

Koc commented Jun 13, 2017

Doctrine is a first-class citizen. Adding this dependency as required helps us avoid many missunderstandings and issues in future.

@garak
Copy link
Collaborator

garak commented Jun 13, 2017

I think it's just a matter of begin clear in docs.
Even if annotations are suggested in first place, they're not technically a requirement

@Koc
Copy link
Contributor Author

Koc commented Jun 13, 2017

Some parts of DI should be rewrited. Currently annotations are required by code, see symfony/recipes-contrib#22 (comment)

@garak
Copy link
Collaborator

garak commented Jun 13, 2017

If so, annotations are actually a requirement.

@Koc Koc deleted the doctrine-annotations-dependency branch June 13, 2017 09:27
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