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

correcting postLink doc block and adding deleteLink #870

Closed
wants to merge 2 commits into from
Closed

correcting postLink doc block and adding deleteLink #870

wants to merge 2 commits into from

Conversation

dereuromark
Copy link
Member

correcting postLink doc block and adding deleteLink as convenience wrapper for DELETE method since those are the two important methods to be used inside the framework.

@ADmad
Copy link
Member

ADmad commented Sep 27, 2012

I don't think its common enough usage to require the new method deleteLink()

@dereuromark
Copy link
Member Author

I was thinking about all those crud action links (index, view, edit actions etc) where there would be this link:
https://github.com/cakephp/cakephp/blob/2.3/lib/Cake/Console/Templates/default/views/view.ctp#L125
instead of the postLink()
since delete actions
https://github.com/cakephp/cakephp/blob/2.3/lib/Cake/Console/Templates/default/actions/controller_actions.ctp#L144
require now either put or delete (which it then would be).

so using it all over the baked views makes this method be used very often and very useful in the end I think.

@ADmad
Copy link
Member

ADmad commented Sep 27, 2012

..require now either put or delete...

.. post or delete ..

@dereuromark
Copy link
Member Author

sry, yes, thats what i meant.
and at some point we can only allow delete in the future, maybe. but for BC it should still allow both, of course.

@markstory
Copy link
Member

I don't like this, it just makes the other HTTP verbs feel left out. We can't really remove postLink() as its there, but I don't think we want putLink() & deleteLink() & patchLink()

@dereuromark
Copy link
Member Author

all right

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