-
-
Notifications
You must be signed in to change notification settings - Fork 438
skip cs related tests in PHP 8 #724
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
Conversation
|
I would rather let them fail until PHP 8 is stable |
|
Agreed but, cs tests block actual tests. Lesser of two evils is to skip cs based tests for now so we can test the actual code base, not the styling of the code. This of course will get reverted when cs tooling is 8x compatible. |
|
I thought we did in #715 🧐 cc @weaverryan |
The difference is that we're going to skip them completely now, instead of running them and telling php-cs-fixer to stop complaining that we're using it on an unsupported version. The reason for that change is that @jrushlow in #725 adds support for php 8 attributes. That forces us to skip php-cs-fixer entirely, because I believe it will explode on the syntax or at least think it's bad cs (but I'm just assuming, I could be wrong). @jrushlow it is true that we should "undo" the parts in #715 in the CI that set the |
|
@jrushlow can you try to remove the env from CI ? |
|
@weaverryan @OskarStark yes and yes. We could leave the CI code in place for that, but ultimately we are not going to need to it with the conditional in the test suite it self to skip cs. For reference to self in the future - when running the test suite locally w/o this PR: |
7d0a4f5 to
cb39b6b
Compare
|
Thanks Jesse! |
skips styling tests in PHP 8 until cs tooling is available