FriendsOfSymfony / FOSHttpCacheBundle

Use the FOSHttpCache library in your Symfony projects
http://foshttpcachebundle.readthedocs.io
Other
430 stars 80 forks source link

chore: add phpstan to CI #622

Closed usu closed 6 months ago

usu commented 6 months ago
usu commented 6 months ago

The new workflow "static" triggers quite a lot of errors "unknown class", which is due to references to dependencies which are dev-dependencies. I could easily fix this by adding a REQUIRE_DEV: "true" to the phpstan-src job. But I wonder if this is correct, or whether all these dependencies should actually be non-dev dependencies...

dbu commented 6 months ago

thanks for looking into this.

dang. good old "optional dependencies" saying hello :-|

i am not sure how optional they really are, a couple places looks to me like it would simply error if thats not available. lets move console and security core & http to the requirements. i would assume most symfony applications use those anyways. and require twig like you require jean-beru - twig really is an optional integration.

usu commented 6 months ago

Fixed as suggested & ready to merge.

Actually console is optional, the services are only loaded when Symfony\Component\Console\Application is available: https://github.com/FriendsOfSymfony/FOSHttpCacheBundle/blob/2.x/src/DependencyInjection/FOSHttpCacheExtension.php#L105 https://github.com/FriendsOfSymfony/FOSHttpCacheBundle/blob/2.x/src/DependencyInjection/FOSHttpCacheExtension.php#L541

However, twig is currently not treated optional. I will fix this in a separate PR.

dbu commented 6 months ago

the remaining failure is for ContextInvalidationLogoutHandler which was symfony 5 only and is being deleted in #618

i merge this as-is with the phpstan failure. thanks for the help!