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

Deprecate CRUDController::configure() method #6751

Merged
merged 2 commits into from
Jan 8, 2021

Conversation

franmomu
Copy link
Member

@franmomu franmomu commented Jan 7, 2021

Subject

In next major version, the controller will be configured with an external call from an event listener instead of using setContainer method.

For more context: #6615

I am targeting this branch, because this is BC.

Changelog

### Deprecated
- Deprecated `CRUDController::configure()` method, to configure the associated admin, you should call `CRUDController::configureAdmin()` instead.

VincentLanglet
VincentLanglet previously approved these changes Jan 7, 2021
@franmomu franmomu requested a review from a team January 7, 2021 14:51
public function setContainer(?ContainerInterface $container = null)
{
$this->container = $container;

$this->configure();
$this->configure('sonata_deprecation_mute');
Copy link
Contributor

@dmaicher dmaicher Jan 7, 2021

Choose a reason for hiding this comment

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

why not call the new configureAdmin method here? then no need to mute the deprecation, or?

Copy link
Member

Choose a reason for hiding this comment

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

In case someone override $this->configure maybe ?

Copy link
Contributor

Choose a reason for hiding this comment

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

ah maybe people have overwritten the configure method 😢 forget it.

dmaicher
dmaicher previously approved these changes Jan 7, 2021
src/Controller/CRUDController.php Outdated Show resolved Hide resolved
src/Controller/CRUDController.php Outdated Show resolved Hide resolved
src/Controller/CRUDController.php Show resolved Hide resolved
src/Controller/CRUDController.php Outdated Show resolved Hide resolved
src/Controller/CRUDController.php Outdated Show resolved Hide resolved
@franmomu franmomu dismissed stale reviews from dmaicher and VincentLanglet via 7e3276c January 8, 2021 12:01
@franmomu franmomu requested a review from phansys January 8, 2021 12:12
phansys
phansys previously approved these changes Jan 8, 2021
@SonataCI
Copy link
Collaborator

SonataCI commented Jan 8, 2021

Could you please rebase your PR and fix merge conflicts?

In next major version, the controller will be configured with an
external call from an event listener instead of using setContainer
method.
@VincentLanglet VincentLanglet merged commit 26e8dfa into sonata-project:3.x Jan 8, 2021
@VincentLanglet
Copy link
Member

Thanks @franmomu

@franmomu franmomu deleted the configure_method branch January 8, 2021 21:11
@franmomu franmomu restored the configure_method branch January 20, 2021 22:59
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.

5 participants