Skip to content
This repository has been archived by the owner on Jan 21, 2020. It is now read-only.

Add case for 'Pimple\Psr11\Container' in HP #270

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

pine3ree
Copy link

@pine3ree pine3ree commented Feb 5, 2019

Add case for new Pimple containerName in HomePageHandler

Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

If this is a bugfix, we'd need a test case for it

@pine3ree
Copy link
Author

pine3ree commented Feb 10, 2019

Hello @Ocramius @xtreamwayz,
I added a test case for the home-page response content (writing it made me also aware of another skipped case, namely the auryn wrapper). I have an issue with test with deps=lowest on travis: zend-servicemanager minimum version is 3.3. In that version the AbstractPluginManager only accepts a Interop\Container\ContainerInterface and not a Psr\Container\ContainerInterface. Any hint for solving this?

(*edited) ...or just skip other containers when using zend-view?

The @dataProvider for the test cases is similar to the ones I found in other test files: i create combination for 2 different install type [flat, modular] x all the supported containers x all the supported renderers x 1 router option (FastRoute) only.
The minimal install type is not tested, as that information is added to json responses in my next PR.

@pine3ree
Copy link
Author

@Ocramius @xtreamwayz

Ok, skipping the zend-view / non zend-sm combinations solved it.

I was wondering if we could just use php imports and class constants like it's done for the router and the renderer and even if it would be worth parametrizing (container|router|template)(Name|Docs) by turning the HomePageHandler.php file into as an install resource itself.

@weierophinney
Copy link
Member

This repository has been closed and moved to mezzio/mezzio-skeleton; a new issue has been opened at mezzio/mezzio-skeleton#4.

@weierophinney
Copy link
Member

This repository has been moved to mezzio/mezzio-skeleton. If you feel that this patch is still relevant, please re-open against that repository, and reference this issue. To re-open, we suggest the following workflow:

  • Squash all commits in your branch (git rebase -i origin/{branch})
  • Make a note of all changed files (`git diff --name-only origin/{branch}...HEAD
  • Run the laminas/laminas-migration tool on the code.
  • Clone mezzio/mezzio-skeleton to another directory.
  • Copy the files from the second bullet point to the clone of mezzio/mezzio-skeleton.
  • In your clone of mezzio/mezzio-skeleton, commit the files, push to your fork, and open the new PR.
    We will be providing tooling via laminas/laminas-migration soon to help automate the process.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants