Skip to content

php8 attributes support for generated controllers #725

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

Merged
merged 1 commit into from
Nov 4, 2020

Conversation

jrushlow
Copy link
Collaborator

@jrushlow jrushlow commented Oct 30, 2020

When using PHP8, Maker Bundle uses route attributes in the generated controllers instead of annotations.

  • make:controller
  • make:crud
  • make:registration-form
  • make:reset-password

As Doctrine/ORM does not currently have PHP8 support, we are unable to test the route functionality for crud, registration-form, & reset-password. No additional tests should be needed when Doctrine is PHP8 compatible.

@weaverryan weaverryan changed the title annotation support for make:controller php8 attributes support for make:controller Nov 1, 2020
@jrushlow jrushlow marked this pull request as ready for review November 2, 2020 16:44
Copy link
Member

@weaverryan weaverryan left a comment

Choose a reason for hiding this comment

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

Minor comments!

I think we should kill/fix ALL the @Route in this one PR:

git grep '@Route' src/Resources/skeleton

It's only 3 other templates I think.

@jrushlow jrushlow force-pushed the feature/controller-annotations branch from 6ac4fae to c6eba1c Compare November 2, 2020 21:22
@jrushlow jrushlow changed the title php8 attributes support for make:controller php8 attributes support for generated controllers Nov 3, 2020
@@ -35,8 +39,13 @@ public function __construct(ResetPasswordHelperInterface $resetPasswordHelper)
/**
* Display & process form to request a password reset.
*
<?php if ($use_attributes) { ?>
*/
#[Route('', name: 'app_forgot_password_request')]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the empty '' path was added on purpose when we first created this maker - but I don't remember exactly why we decided to do this. Will investigate and adjust accordingly under a separate issue.

Copy link
Member

Choose a reason for hiding this comment

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

I also can't remember... it may have been that / here would have made the final URL have a trailing slash - /reset-password/

@@ -35,8 +39,13 @@ public function __construct(ResetPasswordHelperInterface $resetPasswordHelper)
/**
* Display & process form to request a password reset.
*
<?php if ($use_attributes) { ?>
*/
#[Route('', name: 'app_forgot_password_request')]
Copy link
Member

Choose a reason for hiding this comment

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

I also can't remember... it may have been that / here would have made the final URL have a trailing slash - /reset-password/

@weaverryan weaverryan force-pushed the feature/controller-annotations branch from a3e1ea8 to 75532b5 Compare November 4, 2020 15:33
@weaverryan
Copy link
Member

Thank you Jesse!

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.

2 participants