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:controller][make:crud] Make route names start with 'app_' #1007

Merged
merged 2 commits into from
Feb 23, 2022

Conversation

robmeijer
Copy link
Contributor

This PR amends the route names generated by make:controller and make:crud by prepending them with app_.

This matches the route names generated by make:auth and make:registration-form.

@weaverryan
Copy link
Member

This bothers me too :)

@weaverryan
Copy link
Member

@robmeijer could you rebase? I've just merged a PR to get the tests in order (and running for Symfony 6). Thanks :)

@weaverryan
Copy link
Member

Tests are failing due to some sloppy code that I probably wrote awhile ago :). This line -

$serviceId = $serviceId ?? sprintf('maker.maker.%s', Str::asRouteName((new \ReflectionClass($makerClass))->getShortName()));
- should probably change to use asSnakeCase()

@robmeijer
Copy link
Contributor Author

Tests are failing due to some sloppy code that I probably wrote awhile ago :). This line -

$serviceId = $serviceId ?? sprintf('maker.maker.%s', Str::asRouteName((new \ReflectionClass($makerClass))->getShortName()));

  • should probably change to use asSnakeCase()

eergh I was certain I checked usages of that method before changing it. Have pushed the suggested fix.

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 @robmeijer

@jrushlow jrushlow merged commit 187d02e into symfony:main Feb 23, 2022
@robmeijer robmeijer deleted the feature/default-route-names branch February 23, 2022 20:12
@jrushlow jrushlow mentioned this pull request Feb 23, 2022
@alexseif
Copy link

Hello, I wanted to ask you guys what do you think of this proposal? #1334

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

4 participants