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

Adding link(), unlink() shorthand functions #2206

Closed
wants to merge 5 commits into from

Conversation

amreladawy
Copy link

As LINK and UNLINK have been accepted as allowed method, this PR for shorthand routing functions for them.

As LINK and UNLINK have been accepted as allowed method, this PR for shorthand routing functions for them.
@geggleto geggleto added this to the 3.9 milestone Apr 17, 2017
@akrabat
Copy link
Member

akrabat commented Apr 17, 2017

My gut feeling is that link and unlink aren't commonly used enough.

Thoughts, @silentworks, @codeguy ?

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 97.136% when pulling e6fa3c2 on amreladawy:patch-1 into 7d62584 on slimphp:3.x.

Adding unit test functions to cover the new two shorthand functions added for the HTTP methods LINK/UNLINK
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 97.255% when pulling 6e0ae51 on amreladawy:patch-1 into 7d62584 on slimphp:3.x.

@amreladawy
Copy link
Author

Updated the unit test to cover the new functions.

@amreladawy
Copy link
Author

I want to add LINK and UNLINKto the list of methods in the function any() but I am not sure if this would match the approach of avoiding literally listing allowed methods just like PR #2156

@geggleto
Copy link
Member

@akrabat I was thinking the same thing, but I don't see any harm for the short-cuts.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 97.255% when pulling e5fa327 on amreladawy:patch-1 into 7d62584 on slimphp:3.x.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 97.255% when pulling e5fa327 on amreladawy:patch-1 into 7d62584 on slimphp:3.x.

Update unit test for `any()` after adding `LINK` and `UNLINK` to the list of methods.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 97.255% when pulling 09e99cd on amreladawy:patch-1 into 7d62584 on slimphp:3.x.

@silentworks
Copy link
Member

I don't see any harm in writing it the long way by using map, I don't see this becoming a common thing anytime soon and don't feel we need to add a shortcut for it.

@akrabat akrabat closed this Apr 26, 2017
@akrabat akrabat modified the milestones: 3.9.0, 3.9 Aug 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants