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

[Routing] Exposed "utf8" option, defaults "locale" and "format" in configuration #30508

Merged
merged 1 commit into from Mar 20, 2019

Conversation

Projects
None yet
4 participants
@HeahDude
Copy link
Member

HeahDude commented Mar 10, 2019

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets ~
License MIT
Doc PR symfony/symfony-docs#11126

A sibling to #30501, everything is in the title :).

@HeahDude HeahDude force-pushed the HeahDude:routing-utf8 branch 2 times, most recently from c97a98d to 9c45619 Mar 10, 2019

@nicolas-grekas nicolas-grekas added this to the next milestone Mar 10, 2019

@HeahDude HeahDude force-pushed the HeahDude:routing-utf8 branch from 9c45619 to 65eafb7 Mar 10, 2019

@HeahDude

This comment has been minimized.

Copy link
Member Author

HeahDude commented Mar 10, 2019

I've just added support for annotations too :). Doc is coming.

@fabpot

This comment has been minimized.

Copy link
Member

fabpot commented Mar 11, 2019

Why is it useful to expose compiler_class?

@HeahDude

This comment has been minimized.

Copy link
Member Author

HeahDude commented Mar 11, 2019

Well, I was not sure, I did it by consistency. I felt like it was hidden and deserved to shine too :), but no hard feeling, if there is no real use case for it let's just keep utf8 which should increase DX (see related doc PR).

@fabpot

This comment has been minimized.

Copy link
Member

fabpot commented Mar 11, 2019

I would remove it, that's an internal settings that does not need to be exposed.

changes requested + rebase needed

@fabpot
Copy link
Member

fabpot left a comment

compiler_classto be removed

@HeahDude HeahDude force-pushed the HeahDude:routing-utf8 branch from e4114c5 to a1b8c6d Mar 17, 2019

@HeahDude

This comment has been minimized.

Copy link
Member Author

HeahDude commented Mar 17, 2019

Done, I exposed the defaults instead, WDYT?

Status: needs review

@HeahDude HeahDude changed the title [Routing] Exposed "compiler_class" and "utf8" options in configuration [Routing] Exposed "utf8" option and defaults "locale", "format" and "fragment" in configuration Mar 17, 2019

@HeahDude HeahDude changed the title [Routing] Exposed "utf8" option and defaults "locale", "format" and "fragment" in configuration [Routing] Exposed "utf8" option, defaults "locale" and "format" in configuration Mar 17, 2019

@HeahDude HeahDude force-pushed the HeahDude:routing-utf8 branch from 6a65e0b to e1dfa68 Mar 17, 2019

@fabpot

This comment has been minimized.

Copy link
Member

fabpot commented Mar 20, 2019

@HeahDude Can you have a look at the broken tests?

@HeahDude HeahDude force-pushed the HeahDude:routing-utf8 branch from e1dfa68 to 3fd616c Mar 20, 2019

@HeahDude HeahDude force-pushed the HeahDude:routing-utf8 branch from 3fd616c to 2911490 Mar 20, 2019

@fabpot

fabpot approved these changes Mar 20, 2019

@fabpot

This comment has been minimized.

Copy link
Member

fabpot commented Mar 20, 2019

Thank you @HeahDude.

@fabpot fabpot merged commit 2911490 into symfony:master Mar 20, 2019

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Mar 20, 2019

feature #30508 [Routing] Exposed "utf8" option, defaults "locale" and…
… "format" in configuration (Jules Pietri)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[Routing] Exposed "utf8" option, defaults "locale" and "format" in configuration

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | ~
| License       | MIT
| Doc PR        | symfony/symfony-docs#11126

A sibling to #30501, everything is in the title :).

Commits
-------

2911490 [Routing] Exposed "utf8" option, defaults "locale" and "format" in configuration

@HeahDude HeahDude deleted the HeahDude:routing-utf8 branch Mar 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.