-
Notifications
You must be signed in to change notification settings - Fork 430
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
Restore PHP 7.4 compatibility #994
Conversation
The test failures must not be related to this PR, since they are failing even for PHP 8.1 and I didn't touch any functional code. |
Looks like a newer dependency is breaking the tests. Not sure which one ATM. However the once on php74 fail consistently over both lower and high dependencies. Looks like we are using >php74 syntax. Not sure if it's worth removing that syntax again |
here is the new syntax, the |
I just moved the type hint to a docblock, which keeps all of the same functionality in a backwards-compatible way. |
Alright, looks good! Thanks. |
@danepowell Cant figure out why the functional tests fail on windows PHP74 and if it is actually an issue or rather a failure on appveyor. Maybe you have more luck with it? |
That's a mystery. I suspect it's a race condition of some kind in the Windows tests, can you "jiggle the handle" (restart tests a few times) and see if it changes? The log up until the failures is normal, it even shows "GrumPHP is sniffing your code!", so the hooks should be present. |
I'm going to try to spin up a Windows VM locally to reproduce this. @veewee is there a way to get verbose output from tests, i.e. see what all of the fixture creation commands are doing? That would speed up debugging. |
Nailed it. @veewee this is good to go.
After spending hours debugging this, I couldn't agree more 😄 |
Glad to share my pain :) Will look at the details of this PR later |
Thanks for your work on this! Will keep on supporting 7.4 until EOL. |
Wonderful, thanks! |
hey! It looks like this issue may have cropped up again? Across my Magento projects still using 7.4, I can't seem to install (tried on 5/6 different projects and same result each time) - it works fine on my 8.1 projects. phpversion PHP version 7.4.33 is unsupported Thank you for keeping the code quality better. We appreciate your patience and efforts. sam@WCL-L-011:~/sites/project-repo-m2(feature/grumphp-install-stag-2)$ cat grumphp.yml | tail -2 Is this going to be stemming from the fact I'm using Ubuntu on a WSL install? Or was the Windows error in context of running in Windows itself? (which I'm not doing, so not sure if I've got an entirely different issue?) for further context, I'm using the phpro/grumphp-shim composer package, for the dependancies. Sorry for digging up old issue! |
Hello @WCLLTD , This package now has a minimum of PHP 8.0. Nevertheless: GrumPHP has been stable for years now. So you can still use an older version of GrumPHP that match your project's needs. |
hey @veewee thanks for the quick reply! Sorry I did just find a reply confirming this before, sorry for wasting your time here.. I will look at reviewing this further - will likely go back to an older release for the remaining 7.4 projects (which makes my want to move off 7.4 even more!!) thank you! |
#956 introduced PHP 8 compatibility. However, it also removed PHP 7.4 compatibility for no reason that I can see. Since PHP 7.4 and 8 are both still officially maintained, this package should support both. It's important that they are supported simultaneously in a single release so that downstream projects with fixed dependencies can run on both PHP 7.4 and 8.
New Task Checklist:
run()
method readable?run()
method using the configuration correctly?