[5.8] Responsable check moved to Route Pipeline #29263
Closed
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
Reponsable check is now done on the Route Pipeline. This cleans the Base Pipeline class, avoiding unwanted behaviour when using it with Responsable objects. Also added a test to check if a Middleware that returns a Reponsable transforms it to a Response.
seriquynh
approved these changes
Jul 23, 2019
|
I have thought about this situation for a long time and agree with this PR. Not sure it will be merged. |
|
Why not? It may break something? |
|
It's a breaking change on the Pipeline class. It could possibly be done on master branch. |
|
I will redirect my efforts to the master branch then. |
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.
This is a more elegant solution to #24156.
Responsable check is now done on the Route Pipeline. This cleans the Base Pipeline class, avoiding unwanted behaviour when using it with Responsable objects.
Also added a test to check if a Middleware that returns a Responsable (instead of keep going with the pipeline) transforms it to a Response.
Pardon the commits, I was sure has hell I was following the style and had my 5 seconds of fury on the keyboard. Also, I don't know how to combine all the commits into one.