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:crud] Remove / from from index action URL #1583

Merged
merged 3 commits into from
Aug 29, 2024

Conversation

seb-jean
Copy link
Contributor

I propose to remove the / from the index action URL because all other URLs in the src/Resources/skeleton/crud/controller/Controller.tpl.php file do not end with a /. This is more consistent.

@seb-jean seb-jean changed the title Remove / from from index action URL [make:crud] Remove / from from index action URL Jul 30, 2024
@jrushlow jrushlow added Feature New Feature Status: Needs Work Additional work is needed labels Jul 31, 2024
@jrushlow jrushlow added this to the v1.61.0 milestone Jul 31, 2024
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.

Howdy @seb-jean - I cannot see any downsides to this change. We would need to fix the test fixtures by changing the assertion to match the path without the trailing /.

e.g.

$crawler = $client->request('GET', '/sweet/food/');

@jrushlow
Copy link
Collaborator

Note to self: I vaguely remember needing to have a trailing / in the "root" path(s) avoid phpunit test errors - or something to that affect.

e.g. /path/to/resource/ instead of /path/to/resource

But I do the latter in my projects without issue, so this may have been resolved upstream.

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 @seb-jean!

@jrushlow jrushlow added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Work Additional work is needed labels Aug 29, 2024
@jrushlow jrushlow mentioned this pull request Aug 29, 2024
@jrushlow jrushlow merged commit 09089bc into symfony:main Aug 29, 2024
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New Feature Status: Reviewed Has been reviewed by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants