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

Change route fallback action to PATCH instead of POST. Fixes #223 #224

Merged
merged 2 commits into from
Apr 19, 2012

Conversation

asm89
Copy link
Member

@asm89 asm89 commented Apr 5, 2012

This PR introduces a BC break because routes will now default to the PATCH method instead of POST (#223).

The docs are updated to reflect this change. Is there another place where the BC break should be noted?

@asm89
Copy link
Member Author

asm89 commented Apr 5, 2012

ping @everzet :)

@stof
Copy link
Member

stof commented Apr 5, 2012

@asm89 you should add a new section in the UPGRADING file

@lsmith77 as it is a BC break, it should probably not be merged in the 0.6 branch

@everzet
Copy link
Contributor

everzet commented Apr 5, 2012

@asm89 agree with both points from @stof

@Dattaya
Copy link
Contributor

Dattaya commented Apr 5, 2012

About documentation, all these actions now by default accept PATCH method and instead of

Define resource actions
public function lockUserAction($slug)
{} // "lock_user" [POST] /users/{slug}/lock
public function banUserAction($slug)
{} // "ban_user" [POST] /users/{slug}/ban
public function voteUserCommentAction($slug, $id)
{} // "vote_user_comment" [POST] /users/{slug}/comments/{id}/vote

should be:

Define resource actions
public function lockUserAction($slug)
{} // "lock_user" [PATCH] /users/{slug}/lock
public function banUserAction($slug, $id)
{} // "ban_user" [PATCH] /users/{slug}/ban
public function voteUserCommentAction($slug)
{} // "vote_user_comment" [PATCH] /users/{slug}/comments/{id}/vote

@lsmith77
Copy link
Member

@asm89 can you add a note to the UPGRADING guide?

@asm89 asm89 merged commit baff2cf into FriendsOfSymfony:master Apr 19, 2012
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.

5 participants