-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
fix: when running on CLI, two Request objects were used in the system #6089
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to think about this one a bit. On the one hand it is convenient (from a developer standpoint) to have the appropriate Request class anytime the service is called; on the other hand it complicates (from a coding standpoint) our PSR-15 layer even more.
Yes, we need to consider this. |
782e340
to
388c4b5
Compare
Changed the implementation a bit. |
a25153f
to
c31570d
Compare
I'm coming around to this. It is a bit hacky, but our current Request implementation is a mess so that might just need to be the case. Let's see if we can get any more reviews and I'll keep chewing on it. |
I've been thinking about it for a while and can't think of any other way to fix it except this way in the current situation. |
If you've given this plenty of thought I don't feel the need to. At this point pretty much anything would be an improvement and what you've done looks reasonable to me. |
CodeIgniter created CLIRequest, but some classes call Services::request() that returned IncommingRequest.
678b83f
to
bd754ec
Compare
Added changelog. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tiny syntax changes then I'm good with this.
Co-authored-by: MGatner <mgatner@icloud.com>
Co-authored-by: MGatner <mgatner@icloud.com>
Description
Fixes #6086
The current implementation has two Request objects (
IncommingRequest
andCLIRequest
) when running on CLI.Because some classes call
Services::request()
inside and it always returnsIncommingRequest
.When
CLIRequest
is used inCodeIgniter
, developers assume that theCLIRequest
object is used system-wide.This PR makes that so.
Checklist: