Skip to content

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

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

Merged
merged 4 commits into from
Dec 3, 2018

Conversation

fionera
Copy link
Contributor

@fionera fionera commented Nov 28, 2018

See issue: #147

@fionera
Copy link
Contributor Author

fionera commented Nov 28, 2018

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

@acasademont
Copy link
Contributor

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
Copy link
Contributor Author

fionera commented Nov 29, 2018

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

@dnna
Copy link
Contributor

dnna commented Nov 30, 2018

❤️ 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
Copy link
Contributor

andig commented Nov 30, 2018

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

@dnna
Copy link
Contributor

dnna commented Dec 2, 2018

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
Copy link
Contributor

andig commented Dec 2, 2018

Kicked of a rebuild

@dnna
Copy link
Contributor

dnna commented Dec 2, 2018

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
Copy link
Contributor Author

fionera commented Dec 2, 2018

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

@andig andig merged commit a04a3b2 into php-pm:master Dec 3, 2018
@andig
Copy link
Contributor

andig commented Dec 3, 2018

And another one merged. Thanks for PR and review!

@andig andig changed the title [WIP] Watch changes on files that are resources within the container Watch changes on files that are resources within the container Dec 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants