search

php-pm / php-pm-httpkernel

HttpKernel adapter for use of Symfony and Laravel frameworks with PHP-PM
MIT License
246 stars 72 forks source link

Watch changes on files that are resources within the container #148

Closed fionera closed 5 years ago

fionera commented 5 years ago

See issue: https://github.com/php-pm/php-pm-httpkernel/issues/147

fionera commented 5 years ago

The tests are failing because the Container is missing in the Mocks and PHPStan does not like the instanceof on a String

acasademont commented 5 years ago

I'd maybe wrap all that code in a if ($this->debug) condition?

you also have a special function register_file that you can use instead of calling the slaves directly

fionera commented 5 years ago

I still dont see the point why the reloading is blocked when debug is disabled.. But i added the two ideas :D

dnna commented 5 years ago

❤️ this PR (I was actually planning to do something similar because I keep needing this) I'm guessing these tests have been broken for a while, might need a separate PR to fix them.

andig commented 5 years ago

Looks like the failure is unrelated but still new. I would appreciate an independent review of this pr (@dnna ?).

dnna commented 5 years ago

I submitted a PR that should fix the $container not found issue in the tests (#150), but @fionera I think the phpstan issue is related to this PR and should be fixed by you.

Other than that the PR looks fine and works well!

andig commented 5 years ago

Kicked of a rebuild

dnna commented 5 years ago

OK, now it breaks because it tries to access $container->containerDir in the new code. This should be fixable by simply adding a $containerDir property in PHPPM\Tests\SymfonyMocks\Container (not sure why phpunit became more strict all of a sudden with this, but its better). Since this now related with the PR I'm guessing this should be added here

fionera commented 5 years ago

@dnna the phpstan issue is already gone :) I add the $containerDir now...

andig commented 5 years ago

And another one merged. Thanks for PR and review!