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

PSR-7 Implementation #3941

Closed
wants to merge 5 commits into from
Closed

Conversation

MGatner
Copy link
Member

@MGatner MGatner commented Nov 29, 2020

Description
This is a PR for the future. I have been trying to force the PSR-7 transition but I think it is not time yet. The deprecation of the current Message::getHeader() and Message::getHeaders() is the important piece to have ready by 4.1 and those are merged.

This still has some work to be done but should be a decent basis. Pay attention to the new tests/system/HTTP/PSR7 which integrates Http\Psr7Test\Tests, a third-party development library for PSR-7 compliance. These test results will outline the remaining work to be done to make the current classes compliant. There are additional classes that haven't started which should refer to https://github.com/php-fig/http-message/tree/master/src.

One more note: I started with the current HTTP classes as a basis, but whoever works on this down the road might consider starting with an exiting PSR-7 implementation and work on getting it CI4-compliant instead. In the short-term I will be working on tightening the definitions for our HTTP interfaces so this might become a much more realistic process.

Checklist:

  • Securely signed commits
  • Component(s) with PHPdocs
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@MGatner MGatner marked this pull request as draft November 29, 2020 15:01
@lonnieezell
Copy link
Member

@MGatner should we close this?

@MGatner
Copy link
Member Author

MGatner commented Dec 8, 2020

I would like this one to stay because even though it won't work "as is" with the updated HTTP layer it contains the code and tests to take our current HTTP classes to(wards) PSR-7. The other PR really just makes deprecations and splits classes into traits.

@agungsugiarto
Copy link
Contributor

Hello @MGatner, this pr will be merge on future? Im will be happy, because codeigniter step closer with compliance psr. Thanks

@MGatner
Copy link
Member Author

MGatner commented Jan 14, 2021

@agungsugiarto It is one of my goals to have the framework provide a PSR-7 compliant HTTP layer, but there are a few shifts that need to happen before that is possible.

@kenjis
Copy link
Member

kenjis commented Dec 8, 2021

Ref #3230

@kenjis
Copy link
Member

kenjis commented Dec 8, 2021

@MGatner Because of coding style changes, it is difficult to rebase the latest develop.
But it seems we could merge changes like this PR into v4.2.
What do you think?

@MGatner
Copy link
Member Author

MGatner commented Dec 8, 2021

I think I've changed my mind on this. Our current HTTP layer is unequivocally at odds with PSR-7, and in order to move it to compliance we would either need a breaking change or a new namespace.

Because of this I think the best short-term solution is to do what we did with Cache: create a separate, optional repo with wrapper classes that are PSR compliant yet use the framework's HTTP layer.

@lonnieezell
Copy link
Member

@MGatner can this be closed then?

@MGatner MGatner closed this Jan 5, 2022
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.

4 participants