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

Updated docs for features introduced in Symfony 2.4 #621

Merged
merged 2 commits into from
Nov 2, 2015

Conversation

jonny-no1
Copy link
Contributor

Symfony 2.4 introduced constants for HTTP status codes to make code more readable. See: http://symfony.com/blog/new-in-symfony-2-4-using-constants-for-http-status-code.

Also introduced was the request stack with the request service being deprecated for removal in 3.0. See: http://symfony.com/blog/new-in-symfony-2-4-the-request-stack. As the filterAction() doesn't actually depend on the request, I suggest injecting an empty request object.

@makasim
Copy link
Collaborator

makasim commented Jul 21, 2015

we still support symfony 2.3. So there is no way to use new symfony features without providing BC.

@jonny-no1
Copy link
Contributor Author

Ok. The second suggestion still makes sense though: As the filterAction() doesn't actually depend on the request, I suggest injecting an empty request object

@lsmith77 lsmith77 added the State: Reviewing This item is being reviewed to determine if it should be accepted. label Jul 21, 2015
@@ -263,7 +263,7 @@ A simple example showing how to change the filter configuration dynamically.
$this->cacheManager->store($filteredBinary, $path, $filter);
}

return new RedirectResponse($this->cacheManager->resolve($path, $filter), 301);
return new RedirectResponse($this->cacheManager->resolve($path, $filter), Response::HTTP_MOVED_PERMANENTLY);
Copy link
Contributor

Choose a reason for hiding this comment

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

please add a note here to say that for Symfony 2.3 devs should use 301 instead of the constant.

@lsmith77
Copy link
Contributor

lsmith77 commented Nov 1, 2015

ping ..

@jonny-no1
Copy link
Contributor Author

Done. Sorry for the delay

lsmith77 added a commit that referenced this pull request Nov 2, 2015
Updated docs for features introduced in Symfony 2.4
@lsmith77 lsmith77 merged commit c42dd33 into liip:master Nov 2, 2015
@lsmith77 lsmith77 removed the State: Reviewing This item is being reviewed to determine if it should be accepted. label Nov 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants