Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[FrameworkBundle] Add support to 307/308 HTTP status codes in RedirectController#26213
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
nicolas-grekas 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.
thanks for working on this; see comments
| * @throws HttpException In case the route name is empty | ||
| */ | ||
| publicfunctionredirectAction(Request$request,string$route,bool$permanent =false,$ignoreAttributes =false):Response | ||
| publicfunctionredirectAction(Request$request,string$route,bool$permanent =false,$ignoreAttributes =false,bool$keepRequestMethod =false):Response |
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.
for other reviewers: the class is final, so this is not a BC break
| { | ||
| if ('' ==$route) { | ||
| thrownewHttpException($permanent ?410 :404); | ||
| thrownewHttpException($permanent ?Response::HTTP_GONE :Response::HTTP_NOT_FOUND); |
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.
we prefer explicit numbers; please revert, and use numbers everywhere in the PR
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.
@nicolas-grekas Change reverted
fabpot commentedFeb 19, 2018
Thank you@ZipoKing. |
…odes in RedirectController (ZipoKing)This PR was squashed before being merged into the 4.1-dev branch (closes#26213).Discussion----------[FrameworkBundle] Add support to 307/308 HTTP status codes in RedirectController| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#26171| License | MIT| Doc PR |With this PR `RedirectController` will allow to create redirections with use of 307/308 HTTP status codes together with 301/302. Related RFC documents:*https://tools.ietf.org/html/rfc7231*https://tools.ietf.org/html/rfc7538Commits-------64fb5a5 [FrameworkBundle] Add support to 307/308 HTTP status codes in RedirectController
stof commentedFeb 19, 2018
this needs a documentation PR though |
ZipoKing commentedFeb 19, 2018
@stof will prepare some docs PR later today |
…7/308 HTTP status codes (ZipoKing, javiereguiluz)This PR was merged into the master branch.Discussion----------[FrameworkBundle] PR #26213 Document redirections with 307/308 HTTP status codesThis documents changes introduced with pull requestsymfony/symfony#26213Commits-------9468bf9 Reword and added an examplec8ce65d Changes after@Simperfit reviewb3984d0 [PR #26213 Document redirections with 307/308 HTTP status codes
With this PR
RedirectControllerwill allow to create redirections with use of 307/308 HTTP status codes together with 301/302. Related RFC documents: