8000 Use backslash for templates and form by jacquesbh · Pull Request #208 · Sylius/SyliusResourceBundle · GitHub
[go: up one dir, main page]
Skip to content

Use backslash for templates and form #208

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Use backslash for templates and form #208

wants to merge 1 commit into from

Conversation

jacquesbh
Copy link
@jacquesbh jacquesbh commented Oct 30, 2020 •
edited
Loading
Q A
Bug fix? yes, kind of :)
New feature? no
BC breaks? no
Deprecations? no
Related tickets fixes #157
License MIT

Using:

templates: App/Project/Crud

would lead to an error:

[Application] Oct 30 14:49:50 |CRITICA| PHP    Uncaught Exception: The file "alias: harvest.harvest_project section: admin templates: Admin/Project" does not exist in /Users/jacques/Sites/operandi/apps/sylius/config/../src/Resources/config/app/routing/admin.yaml (which is being imported from "/Users/jacques/Sites/operandi/apps/sylius/config/routes.yaml").

I tried everything until I saw the use of backslash on another project… I tried and it works fine.

So, I've updated the documentation.

Also, the form is incorrect as well, but not related, I think since we use backslashes for PHP classes.

@jacquesbh jacquesbh requested a review from a team as a code owner October 30, 2020 14:51
@pamil
Copy link
Contributor
pamil commented Nov 5, 2020

Could we somehow add a test for that convention so that we can be sure it works? We've got a test application in src/Bundle/test.

@loic425
Copy link
Member
loic425 commented Nov 5, 2020

@jacquesbh @pamil To follow Symfony best practices, we should use lowercase for templates:
templates: admin\book

But this is a very old bug and I think we can fix it instead. I'm not a big fan to have a template path with backslashes.

@jacquesbh
Copy link
Author

Could we somehow add a test for that convention so that we can be sure it works? We've got a test application in src/Bundle/test.

@pamil Yes. But it requires more time. So this PR will just wait for it :p.

To follow Symfony best practices, we should use lowercase for templates:
templates: admin\book
But this is a very old bug and I think we can fix it instead. I'm not a big fan to have a template path with backslashes.

@loic425 I agree with you it's an old bug. But fixing it will lead to BC breaks, won't it? Since it will break a lot of applications and bundles using the backslashes formatted paths.

Sorry, something went wrong.

@pamil
Copy link
Contributor
pamil commented Dec 9, 2020

It shouldn't be needed after symfony/symfony#39142 is released 🚀

@pamil pamil closed this Dec 9, 2020
@loic425
Copy link
Member
loic425 commented Dec 9, 2020

It shouldn't be needed after symfony/symfony#39142 is released 🚀

Great news!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Error parsing 'templates' property in routing.yml for new Resource
3 participants
@jacquesbh @pamil @loic425
0