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.

Sign up for GitHub

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

Jump to bottom

Fixes for Pimple 3 use #317

Merged
merged 3 commits into from
Sep 2, 2020
Merged

Fixes for Pimple 3 use #317

merged 3 commits into from
Sep 2, 2020

Conversation

glensc
Copy link
Contributor

@glensc glensc commented Sep 2, 2020 •
edited
Loading

  • Allow lower Pimple version for PHP 7.0
  • Fix tests: Ensure Run Controller is a factory in the container (otherwise runTest fails because it expects a new instance in each test)
  • Add missing parent::__construct for the container (otherwise factories and protected were not initialized)
- pimple/pimple v3.3.0 requires php ^7.2.5 -> your PHP version (7.0.33) does not satisfy that requirement.

refs:

@glensc glensc self-assigned this Sep 2, 2020
@glensc glensc changed the title Allow Lower pimple for php 7.0 Allow lower Pimple for php 7.0 Sep 2, 2020
@glensc
Add missing parent::__construct for container
4c110d7 
Otherwise factories and protected were not initialized
@glensc glensc changed the title Allow lower Pimple for php 7.0 Fixes for Pimple 3 use Sep 2, 2020
@glensc
Ensure Controllers are factory in container
0277406 
Otherwise runTest and watchTest fail because they expects new instance
in each test. For sake of clarity use same pattern for all controllers.
In fact, the controllers are not reused when application itself is run.
@glensc glensc merged commit 2fb9d54 into perftools:master Sep 2, 2020
@glensc glensc deleted the lower-pimple branch September 2, 2020 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@glensc glensc

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@glensc