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

T12295 psr 11 #13681

Merged
merged 8 commits into from
Dec 23, 2018
Merged

T12295 psr 11 #13681

merged 8 commits into from
Dec 23, 2018

Conversation

niden
Copy link
Member

@niden niden commented Dec 22, 2018

Hello!

In raising this pull request, I confirm the following (please check boxes):

  • I have read and understood the Contributing Guidelines?
  • I have checked that another pull request for this purpose does not exist.
  • I wrote some tests for this PR.

Created a new "proxy" class Phalcon\Container. This class implements PSR-11 and can be used instead of the DI container. Note that you still have to create the DI container and set all services into it and then inject that container in this container (I got dizzy writing this).

This is not the full implementation, a "proxy" for now - more on that in a future version.

Thanks

* 4.0.x: (33 commits)
  [4.0.x] - Another correction to the test
  Corrected test
  Fixed tests
  Removed obsolete file
  Fixed the method signature
  PHPCS fix
  Fixed tabs
  Corrected tests
  [#12833] - Updated the changelog
  [#12833] - Deleted obsolete tests
  [#12833] - PHPCS fixes
  [#12833] - Corrections to the manager and test (cleanup superglobal on destroy)
  [#12833] - Fixes and corrections to the tests
  [#12833] - Corrections to the tests and files adapter
  [#12833] - Fixed tests; Added exception in session for non valid handler
  [#12833] - Full tests for Session\Adapter; Adjustments to the environment; Test stubs
  [#12833] - Corrections and adding files adapter tests
  [#12833] - Correction to the redis session adapter; Work on the test traits
  [#12833] - Setup default session_save path to /tmp
  [#12833] - Cleanup for tests
  ...
@niden niden added the enhancement Enhancement to the framework label Dec 22, 2018
@niden niden requested a review from sergeyklay December 22, 2018 23:34
@niden
Copy link
Member Author

niden commented Dec 22, 2018

cc @sergeyklay

$container = new Di();
$class = ContainerInterface::class;
$actual = new Container($container);
$I->assertInstanceOf($class, $actual);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be simplified, eg

$I->assertInstanceOf(ContainerInterface::class, new Container(new Di));

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like to keep things separate in the test; It makes reading a bit easier and discourages people writing an assertion that is long and difficult to understand.

use Psr\Container\ContainerInterface;
use Phalcon\DiInterface;

final class Container implements ContainerInterface
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need this as final

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, Let me correct that

sergeyklay
sergeyklay previously approved these changes Dec 22, 2018
Copy link
Contributor

@sergeyklay sergeyklay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@niden niden merged commit dd2192a into phalcon:4.0.x Dec 23, 2018
@niden niden deleted the T12295-PSR-11 branch March 29, 2019 23:43
@niden niden added the documentation Documentation required label Apr 9, 2019
@niden niden added 4.0 and removed documentation Documentation required labels Dec 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement to the framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants