Skip to content

Consider renaming pattern to path in the routing component #5989

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

Closed
fabpot opened this issue Nov 12, 2012 · 10 comments
Closed

Consider renaming pattern to path in the routing component #5989

fabpot opened this issue Nov 12, 2012 · 10 comments
Labels

Comments

@fabpot
Copy link
Member

fabpot commented Nov 12, 2012

As of today, we have two patterns: one for the path (pattern) and another one for the host (hostname_pattern).

In #3378, it was suggested to rename pattern to path and hostname_pattern to hostname.

That's fine by me, but of course, we will keep pattern support even after the LTS release (which should not be a problem). The documentation for 2.2 will of course only use the new name.

What others think?

@Seldaek
Copy link
Member

Seldaek commented Nov 12, 2012

👍 from me, hoping that it doesn't create too much confusion in the future.

@alvarezmario
Copy link
Contributor

👍 for the change. I don't really see the need of the _patern suffix just to clarify that you can use variables and requirements. It only need to be put on the docs and nothing more. You can also use variables and requirements in _locale and is not named _locale_pattern

@Tobion
Copy link
Contributor

Tobion commented Nov 12, 2012

👍 as it makes it much things much clearner. Otherwise one does't know what pattern belongs to (could be anything).

@Tobion
Copy link
Contributor

Tobion commented Nov 12, 2012

@Nomack84: No, you cannot "use variables in _locale". _locale is a variable for itself. So your argument with _locale_pattern is not valid.

@alvarezmario
Copy link
Contributor

@Tobion right :) but even so, the docs are enough.

@schmittjoh
Copy link
Contributor

I haven't really seen any confusion around pattern, but since we would
still keep BC by supporting the old name, I'm -0

On Mon, Nov 12, 2012 at 2:21 PM, Mario A. Alvarez Garcia <
notifications@github.com> wrote:

@Tobion https://github.com/Tobion right :) but even so, the docs are
enough.


Reply to this email directly or view it on GitHubhttps://github.com//issues/5989#issuecomment-10286679.

@Tobion
Copy link
Contributor

Tobion commented Nov 12, 2012

@schmittjoh until now, there could not be any confusion as there was only one "pattern" available. But now that we have 2, the current naming is really confusing.

@schmittjoh
Copy link
Contributor

I'm aware of the hostname change, and personally I don't have that confusion.

@Tobion
Copy link
Contributor

Tobion commented Nov 12, 2012

I opened another RFC that improves the route definition: #5990

@henrikbjorn
Copy link
Contributor

👍

fabpot added a commit that referenced this issue Jan 15, 2013
This PR was merged into the master branch.

Commits
-------

9fc7def added the UPGRADE file for Symfony 3.0
e84cad2 [Routing] updated CHANGELOG
65eca8a [Routing] added new schemes and methods options to the annotation loader
5082994 [Routing] renamed pattern to path
b357caf [Routing] renamed hostname pattern to just hostname
e803f46 made schemes and methods available in XmlFileLoader
d374e70 made schemes and methods available in YamlFileLoader
2834e7e added scheme and method setter in RouteCollection
10183de make scheme and method requirements first-class citizen in Route

Discussion
----------

Routing options

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #5989, #5990, #6049
| License       | MIT

In #5989, it has unanimously been decided to renamed `hostname_pattern` to `hostname` and `pattern` to `path`. That makes a lot of sense and I would like to do the renaming now as `hostname_pattern` is new in Symfony 2.2, so I'd like to avoid breaking BC just after the release. As we are modifying the route options, I've also included changes introduced by @Tobion in #6049 which were discussed in #5990.

As everything is BC, I think it's wise to include that in 2.2. What do you think?

---------------------------------------------------------------------------

by Tobion at 2013-01-14T18:25:53Z

I agree it should be done in 2.2. Thanks for working on it.

---------------------------------------------------------------------------

by vicb at 2013-01-14T23:11:12Z

@fabpot "Everything is BC" until it breaks BC in 3.0, that's why I'd like to see [deprecations in PR summary](symfony/symfony-docs#2116) what do you think ?

---------------------------------------------------------------------------

by vicb at 2013-01-14T23:16:40Z

it would also be great to update the CHANGELOG with deprecations (it could also help people answering your question)

---------------------------------------------------------------------------

by fabpot at 2013-01-15T07:07:03Z

@vicb: I've just updated the CHANGELOG and created the UPGRADE file for 3.0.

---------------------------------------------------------------------------

by vicb at 2013-01-15T07:15:32Z

@fabpot thanks.
@fabpot fabpot closed this as completed Jan 15, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants