Fix routes options to be recognizable via used HTTP request method #3751
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
samsonasik
reviewed
Oct 10, 2020
samsonasik
reviewed
Oct 10, 2020
samsonasik
reviewed
Oct 10, 2020
samsonasik
reviewed
Oct 10, 2020
MGatner
requested changes
Oct 10, 2020
There was a problem hiding this comment.
I like this. I agree with @samsonasik's changes, also... I know getRoutes() has an unspecified variable type on $verb but other things will break if it is not string|null so I think we should go ahead and update the function declaration to be consistent.
Co-authored-by: Abdul Malik Ikhsan <samsonasik@gmail.com> Co-authored-by: John Paul E. Balandan, CPA <51850998+paulbalandan@users.noreply.github.com>
|
Thanks for your code suggestions guys. |
MGatner
approved these changes
Oct 11, 2020
|
👍 i was writing a bug report just at this moment and saw the merge while digging for the buggy code 🎆 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes routes options to be recognizable via used HTTP request method. Right now routes options are shared - one instance per route. It leads to problems like: wrong named route name returned or firing incorrect filter.
I don't think this will introduce any BC, rather will fix buggy behavior, but if anyone notices the problem please let me know.
Fixes #3700
Fixes #3733
Checklist: