PHP 7.2 compatibility
We need to move toward full PHP 7.2 compatibility across our codebase. The target for this is late spring; I'll put this ticket into the 1.13 milestone for the time being.
The number one concern is eliminating fatal errors. Between our current version (5.6.x) and 7.2, there are a number of hard deprecations and removals. See https://secure.php.net/manual/en/migration72.php and the corresponding 7.1 and 7.0 pages for more info.
Much of this work can probably be done with an existing linter, like https://github.com/sstalle/php7cc. The linter will need to be configured to ignore certain parts of the codebase, such as bbPress legacy bundled with BP.
Where possible/convenient/worthwhile, we'll make an attempt to pass compatibility patches to upstream plugin developers. When plugins are abandoned, we can patch locally and call it good enough. In some cases, we may take this opportunity to disable or remove certain plugins/themes. We'll take this on a case-by-case basis.
We'll also likely need a tool that enforces PHP 7.2 compatibility across the codebase after updates. We don't want to patch a plugin only to have a future update to that plugin reintroduce an incompatibility. One idea here is to run a linter in a Travis CI task, which would email us when bad code is committed. But this would require some changes to our deployment process, since we'd need time for the linting to complete.
We should scour the internet to look for people who have done this with large WordPress codebases, as it's likely that we can borrow much or all of others' work.
Let's use this ticket to start gathering ideas.
#2 Updated by Boone Gorges over 3 years ago
I spent some time working on this issue today. Here's what I've done.
1. I adapted an existing script to do the linting. The linting is done using the local version of PHP, so in order to catch PHP 7.2 issues, it must be run against PHP 7.2. The script lives at developer/lint-php.php. https://github.com/cuny-academic-commons/cac/blob/a41d800f3a2f69d030756f5e66a2a08c3563cca7/developer/lint-php.php
2. I configured a CircleCI integration with our GitHub repo. https://circleci.com/gh/cuny-academic-commons/cac/ This integration runs the linter script after each commit to the repo. Failures result in email notifications to me. This will have to be modified once we decide exactly when the linting ought to take place. Maybe only against the master branch, since that's the release branch. I'm using CircleCI because they don't charge to run jobs on a private repo, while Travis does.
3. I've compiled a spreadsheet with the results of the initial linting run: https://docs.google.com/spreadsheets/d/1n2HcDTiCrTDt9wTmMotU9xXxHTin5MNOmPVl_Pwmov0/edit?usp=sharing The ones marked "Abandoned" should just be fixed directly in our repo by one of the dev team, and then checked off in that column. The others require some other sort of action: getting new copies of a premium plugin, etc.
#4 Updated by Boone Gorges over 3 years ago
- Target version changed from 1.13 to 1.14
I've fixed all the items in the spreadsheet https://docs.google.com/spreadsheets/d/1n2HcDTiCrTDt9wTmMotU9xXxHTin5MNOmPVl_Pwmov0/edit#gid=0 and our build is now green on CircleCI: https://circleci.com/gh/cuny-academic-commons
We still have some work to do here on processes, so that we can ensure that we don't reintroduce PHP 7.2 incompatibilities.
In the meantime, I'll follow up with Lihua to see where the server stuff plans, and so that we can start coming up with a clear schedule. I'm going to bump this out of the 1.13 milestone, since it's not directly affected by the release.
#5 Updated by Raymond Hoh over 3 years ago
Caught a PHP 7.2 deprecated call in
social-paper - https://github.com/cuny-academic-commons/cac/commit/3aba5ecb49cbef06482dc8d91679ef4d88a75cd2
Boone - Does our linter pick up git submodules? It looks like your linter script should work, but I'm not sure.
#6 Updated by Boone Gorges over 3 years ago
The linter is set up to catch fatals only. I'm sure there are thousands of instances of code that will trigger notices.
But yes, the linter will run on anything that's in the local directory. So as long as the submodules are checked out, they'll be linted. I just added a couple lines to our circleci config to ensure that submodules do get initialized. I'll verify after it's run.