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

Implementing HTTP 405 #1335

Closed
wants to merge 23 commits into from
Closed

Implementing HTTP 405 #1335

wants to merge 23 commits into from

Conversation

Drunpy
Copy link

@Drunpy Drunpy commented Jun 11, 2020

This PR is the same as #1276 but without formatting changes. Solves #1224

@SergioBenitez
Copy link
Member

Please see this part of my previous comment:

My concern is the performance impact that this will have. It's very nice to be able to filter by method: it means we use it as a prefix. We should not remove this from the router. At worst, we should simply iterate through all possible methods, checking to see if there's a match, in case no route was found. This means we take the performance hit only if there's no match initially.

@Drunpy
Copy link
Author

Drunpy commented Jun 16, 2020

Sure, I will implement this way.

@SergioBenitez SergioBenitez marked this pull request as draft June 22, 2020 12:06
@Drunpy
Copy link
Author

Drunpy commented Jul 3, 2020

Hey @igalic thanks for your valuable comments, I've done some modifications based on these!

@SergioBenitez
Copy link
Member

@Drunpy Note that to get this PR accepted, in addition to the changes requested, you'll need to rebase on master, not merge it in.

@SergioBenitez
Copy link
Member

I'm still in favor of this, but simply closing due to inactivity.

@peng1999 peng1999 mentioned this pull request Dec 30, 2020
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