Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Routing] Allow to set name prefixes from the configuration#25178
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
0875723 toe0a8690Compare| publicfunctiontestAddNamePrefix() | ||
| { | ||
| $collection =newRouteCollection(); | ||
| $collection->add('foo',$foo =newRoute('/foo')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
please also add an existingapi_foo route, to be sure that the prefixing does not delete this route due to overriding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Good catch, added.
stof commentedNov 27, 2017
Does the PHP DSL also need this feature, or is it already available in it ? |
841d5ee toa4c836cComparesroze commentedNov 28, 2017
@stof it was't. I've added it to the trait. |
sroze commentedNov 30, 2017
Status: Needs review |
| } | ||
| /** | ||
| * Add a prefix to the name of all the routes within the collection. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Adds
| } | ||
| /** | ||
| * Add a prefix to the name of all the routes within in the collection. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Adds
| /** | ||
| * Add a prefix to the name of all the routes within in the collection. | ||
| * | ||
| * @param string $prefix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Should be removed
| /** | ||
| * Add a prefix to the name of all the routes within the collection. | ||
| * | ||
| * @param string $prefix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Should be removed
| * | ||
| * @return $this | ||
| */ | ||
| finalpublicfunctionaddNamePrefix(string$prefix) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
: self to fully get rid of the phpdoc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Well, we still need the description I'd say, as it's meant to be used by developers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Getting rid of@return $this would be nice though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
but... on the trait I'm not sure if this makes sense... as (at least in my mind)self refers to the trait here, not the classuseing the trait.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
There is no such thing as referring to a trait. PHP merges traits into the class, so self will always point to the correct class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I was talking conceptually, by reading this. But if you two believe it's better just to putself within the trait, I'm happy with it as I don't mind that much tbh. Changed 👍
a4c836c to9f7c4e7Compare
sroze left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Updated based on@fabpot's comments.
| * | ||
| * @return $this | ||
| */ | ||
| finalpublicfunctionaddNamePrefix(string$prefix) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Well, we still need the description I'd say, as it's meant to be used by developers
9f7c4e7 todd15433CompareAdd the `addNamePrefix` method to the PHP trait
dd15433 toe895402Comparefabpot commentedDec 2, 2017
Thank you@sroze. |
…ation (sroze)This PR was squashed before being merged into the 4.1-dev branch (closes#25178).Discussion----------[Routing] Allow to set name prefixes from the configuration| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#19612| License | MIT| Doc PR | øThis allows setting name prefixes to routes while importing them. Typically, we can then import multiple times a similar file. This was originally requested by 🎸@chrisguitarguy in#19612```yamlapp: resource: ../controller/routing.ymlapi: resource: ../controller/routing.yml name_prefix: api_ prefix: /api``````xml<?xml version="1.0" encoding="UTF-8" ?><routes xmlns="http://symfony.com/schema/routing" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/routinghttp://symfony.com/schema/routing/routing-1.0.xsd"> <import resource="../controller/routing.xml" /> <import resource="../controller/routing.xml" prefix="/api" name-prefix="api_" /></routes>```Commits-------880d7e7 [Routing] Allow to set name prefixes from the configuration
…rekas)This PR was merged into the 4.1-dev branch.Discussion----------[Routing] Fix name-prefixing when using PHP DSL| Q | A| ------------- | ---| Branch? | master| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -Fixes bad implem merged in#25178Commits-------0053eee [Routing] Fix name-prefixing when using PHP DSL
Uh oh!
There was an error while loading.Please reload this page.
This allows setting name prefixes to routes while importing them. Typically, we can then import multiple times a similar file. This was originally requested by 🎸@chrisguitarguy in#19612