Skip to content

disable the deprecated sensio_framework_extra.router.annotations #790

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

Closed
wants to merge 2 commits into from
Closed

disable the deprecated sensio_framework_extra.router.annotations #790

wants to merge 2 commits into from

Conversation

christickner
Copy link
Contributor

fixes #786

famework.annotations still needs to be true due to the validation: { enable_annotations: true } in framework.yaml.

Should I move the framework.annotations: true to the framework.yaml file instead? It seems more appropriate.

@javiereguiluz
Copy link
Member

Not sure what's the best choice. Let's ask for help to some expert developers who are active in this repo: @voronkovich @bocharsky-bw @yceruto. Thanks!

@stof
Copy link
Member

stof commented Apr 25, 2018

Why do we have framework_extra.yml configuring framework ? this looks weird (and useless, as FrameworkBundle already enables the annotation layer properly based on the fact that doctrine/annotations is installed)

@voronkovich
Copy link
Contributor

I agree with @stof. I think we should just delete the framework_extra.yml file.

it is not necessary to configure the "framework" bundle to enable annotations in this way (and this is the wrong file to put that config anyway)
@christickner
Copy link
Contributor Author

agreed. removed the "framework" config from the file here: a063862

@voronkovich note that we can't delete the file entirely, as we have to explicitly disable sensio_framework_extra.router.annotations for this issue to be solved.

All: should I squash, rebase on master, and force-push, or are we okay as is?

@javiereguiluz
Copy link
Member

@christickner thanks for your work on this! There's no need to squash or rebase because the tool we use to merge things do that automatically. Cheers!

@javiereguiluz
Copy link
Member

...and it's merged! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Our current framework_extra.yaml config is deprecated since Symfony 4.1
4 participants