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

Make some method of the CRUDController final #7252

Merged
merged 3 commits into from
Jun 16, 2021

Conversation

VincentLanglet
Copy link
Member

@VincentLanglet VincentLanglet commented Jun 12, 2021

Subject

I am targeting this branch, because BC-break.

I let as non final

Changelog

### Changed
- Add final to multiple method of CRUDController

@VincentLanglet VincentLanglet changed the base branch from 3.x to master June 12, 2021 20:36
@VincentLanglet VincentLanglet added this to the 4.0 milestone Jun 12, 2021
@VincentLanglet VincentLanglet marked this pull request as ready for review June 13, 2021 18:52
*/
protected function renderWithExtraParams(string $view, array $parameters = [], ?Response $response = null): Response
final protected function renderWithExtraParams(string $view, array $parameters = [], ?Response $response = null): Response
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't we add @final in 3.x to these methods to warn users?

Copy link
Member Author

Choose a reason for hiding this comment

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

I will do. I prefer to first decide what should be final.

src/Controller/CRUDController.php Outdated Show resolved Hide resolved
@VincentLanglet VincentLanglet requested review from phansys and a team June 15, 2021 23:13
@VincentLanglet VincentLanglet merged commit 458cf9d into sonata-project:master Jun 16, 2021
@VincentLanglet VincentLanglet deleted the finalCrud branch June 16, 2021 09:10
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.

4 participants