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:registration] Make router optional in MakeRegistrationForm constructor #1227

Merged
merged 1 commit into from
Nov 8, 2022

Conversation

odolbeau
Copy link
Contributor

In a console application, I have the following configuration:

framework:
  router: false

As the MakeRegistrationForm class need a RouterInterface to be constructed, I get the following error: Symfony\Bundle\MakerBundle\Maker\MakeRegistrationForm::__construct(): Argument #3 ($router) must be of type Symfony\Component\Routing\RouterInterface, null given

This PR aim to remove this dependency.

@@ -111,6 +111,10 @@ public function interact(InputInterface $input, ConsoleStyle $io, Command $comma
{
$interactiveSecurityHelper = new InteractiveSecurityHelper();

if (null === $this->router) {
throw new RuntimeCommandException('Router have been explicitely disabled in your configuration. This command needs to use the router.');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Help welcome to choose a clearer message.

Copy link
Member

Choose a reason for hiding this comment

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

Looks good enough to me - especially for this edge-case

@weaverryan
Copy link
Member

Ah, so you can't run maker at all in your console app? That makes sense - I don't normally think about the router service not being available, but that seems legal to me.

private DoctrineHelper $doctrineHelper,
private ?RouterInterface $router = null,
Copy link
Member

Choose a reason for hiding this comment

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

Class is internal, so no problems changing args 👍

@@ -111,6 +111,10 @@ public function interact(InputInterface $input, ConsoleStyle $io, Command $comma
{
$interactiveSecurityHelper = new InteractiveSecurityHelper();

if (null === $this->router) {
throw new RuntimeCommandException('Router have been explicitely disabled in your configuration. This command needs to use the router.');
Copy link
Member

Choose a reason for hiding this comment

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

Looks good enough to me - especially for this edge-case

@weaverryan weaverryan added the Status: Reviewed Has been reviewed by a maintainer label Oct 25, 2022
@jrushlow jrushlow changed the title Make router optional in MakeRegistrationForm constructor [make:registration] Make router optional in MakeRegistrationForm constructor Nov 8, 2022
@jrushlow jrushlow added the Bug Bug Fix label Nov 8, 2022
Copy link
Collaborator

@jrushlow jrushlow left a comment

Choose a reason for hiding this comment

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

Thank you @odolbeau

@jrushlow jrushlow merged commit 9b78302 into symfony:main Nov 8, 2022
@jrushlow jrushlow mentioned this pull request Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Bug Fix Status: Reviewed Has been reviewed by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants