Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[FrameworkBundle][DX] Improving the redirect config when using RedirectController#33217

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

Merged
yceruto merged 1 commit intosymfony:4.4fromyceruto:simpler_redirect_config
Aug 23, 2019

Conversation

@yceruto
Copy link
Member

@ycerutoyceruto commentedAug 16, 2019•
edited
Loading

QA
Branch?4.4
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets-
LicenseMIT
Doc PRsymfony/symfony-docs#12189

follow-up#24637

Before:

# config/routes.yamldoc_shortcut:path:/doccontroller:Symfony\Bundle\FrameworkBundle\Controller\RedirectController::redirectActiondefaults:route:'doc_page'legacy_doc:path:/legacy/doccontroller:Symfony\Bundle\FrameworkBundle\Controller\RedirectController::urlRedirectActiondefaults:path:'https://legacy.example.com/doc'

After:

# config/routes.yamldoc_shortcut:path:/doccontroller:Symfony\Bundle\FrameworkBundle\Controller\RedirectControllerdefaults:route:'doc_page'legacy_doc:path:/legacy/doccontroller:Symfony\Bundle\FrameworkBundle\Controller\RedirectControllerdefaults:path:'https://legacy.example.com/doc'

See more before/after configs (XML, PHP) in doc PRsymfony/symfony-docs#12189

ismail1432, ro0NL, linaori, and javiereguiluz reacted with thumbs up emojilinaori reacted with hooray emoji
@carsonbotcarsonbot added Status: Needs Review FrameworkBundle DXDX = Developer eXperience (anything that improves the experience of using Symfony) Feature labelsAug 16, 2019
@ycerutoycerutoforce-pushed thesimpler_redirect_config branch from6ce11cc to93e69eaCompareAugust 16, 2019 21:07
@ycerutoyceruto added this to thenext milestoneAug 16, 2019
@ycerutoycerutoforce-pushed thesimpler_redirect_config branch from93e69ea tobea281dCompareAugust 17, 2019 03:43
@wouterj
Copy link
Member

👍 What about deprecating the other two methods? (in favor of making them private/protected in Symfony 5.0)

ro0NL reacted with thumbs up emojiandrew-demb reacted with thumbs down emoji

@nicolas-grekas
Copy link
Member

I don't think deprecating is worth the trouble it will put on users.

wouterj and yceruto reacted with thumbs up emoji

@linaori
Copy link
Contributor

From a DX perspective, this would be really amazing if we could make something like this:

# config/routes.yamldoc_shortcut:path:/doccontroller:RedirectControllerdefaults:route:'doc_page'legacy_doc:path:/legacy/doccontroller:RedirectControllerdefaults:path:'https://legacy.example.com/doc'

Or perhaps:

# config/routes.yamldoc_shortcut:path:/docredirect:route:'doc_page'legacy_doc:path:/legacy/docredirect:path:'https://legacy.example.com/doc'
Tobion, maxhelias, and ro0NL reacted with thumbs down emoji

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

We should throw also when both settings are defined at the same time.

yceruto reacted with thumbs up emoji
@ycerutoycerutoforce-pushed thesimpler_redirect_config branch frombea281d todeada37CompareAugust 19, 2019 11:20
thrownew \RuntimeException('Ambiguous redirection settings, use the "path" or "route" parameter, not both.');
}

return$this->redirectAction($request,$p['route'],$p['permanent'] ??false,$p['ignoreAttributes'] ??false,$p['keepRequestMethod'] ??false,$p['keepQueryParams'] ??false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

for all the common parameters, you could make them arguments of the controller instead, to let Symfony bind them (with any logic applied by argument resolver, as done for other methods) and take the default values from the method.

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I'm not sure if it's worth it just for two common parameters.

@ycerutoycerutoforce-pushed thesimpler_redirect_config branch from2ee8baf to33fd9b0CompareAugust 20, 2019 19:41
@ycerutoycerutoforce-pushed thesimpler_redirect_config branch from33fd9b0 tocb3cafaCompareAugust 20, 2019 19:56
@ycerutoycerutoforce-pushed thesimpler_redirect_config branch fromcb3cafa to0ebb469CompareAugust 23, 2019 21:46
yceruto added a commit that referenced this pull requestAug 23, 2019
…en using RedirectController (yceruto)This PR was merged into the 4.4 branch.Discussion----------[FrameworkBundle][DX] Improving the redirect config when using RedirectController| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | -| License       | MIT| Doc PR        |symfony/symfony-docs#12189follow-up#24637**Before:**```yaml# config/routes.yamldoc_shortcut:    path: /doc    controller: Symfony\Bundle\FrameworkBundle\Controller\RedirectController::redirectAction    defaults:        route: 'doc_page'legacy_doc:    path: /legacy/doc    controller: Symfony\Bundle\FrameworkBundle\Controller\RedirectController::urlRedirectAction    defaults:        path: 'https://legacy.example.com/doc'```**After:**```yaml# config/routes.yamldoc_shortcut:    path: /doc    controller: Symfony\Bundle\FrameworkBundle\Controller\RedirectController    defaults:        route: 'doc_page'legacy_doc:    path: /legacy/doc    controller: Symfony\Bundle\FrameworkBundle\Controller\RedirectController    defaults:        path: 'https://legacy.example.com/doc'```See more before/after configs (XML, PHP) in doc PRsymfony/symfony-docs#12189Commits-------0ebb469 Improving redirect config when using RedirectController
@ycerutoyceruto merged commit0ebb469 intosymfony:4.4Aug 23, 2019
@ycerutoyceruto deleted the simpler_redirect_config branchAugust 23, 2019 21:48
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull requestSep 2, 2019
…ng RedirectController (yceruto)This PR was merged into the 4.4 branch.Discussion----------[FrameworkBundle][DX] Improving redirect config when using RedirectControllerUpdate according tosymfony/symfony#33217Commits-------f59c61f [DX] Improving redirect config
@nicolas-grekasnicolas-grekas removed this from thenext milestoneOct 27, 2019
@nicolas-grekasnicolas-grekas added this to the4.4 milestoneOct 27, 2019
This was referencedNov 12, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@stofstofstof left review comments

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

+1 more reviewer

@TobionTobionTobion approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

DXDX = Developer eXperience (anything that improves the experience of using Symfony)FeatureFrameworkBundleStatus: Reviewed

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

7 participants

@yceruto@wouterj@nicolas-grekas@linaori@stof@Tobion@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp