PHP Min Tests#495
Conversation
namithj
commented
May 15, 2026
namithj
commented
- PHP Min Tests
- Update DID Manager Wordpress to version 0.0.5 or above
1) PHP Min Tests 2) Update DID Manager Wordpress to version 0.0.5 or above Signed-off-by: Namith Jawahar <namith.jawahar@gmail.com>
kasparsd
left a comment
kasparsd
left a comment
There was a problem hiding this comment.
The dependency bump looks good.
Re: php compatibility checks -- could we leave that to a separate changeset that uses our existing CI test matrix to run the existing php-compatability checks (or introduce php-parallel-lint to do it quicker)? The current onliner relies on a lot of system-level tools which aren't available (or work differently) across operating systems.
| "@format:phpcs", | ||
| "@format:phpstan" | ||
| ], | ||
| "detect-min-php": "sh -c 'for v in 5.4 5.5 5.6 7.0 7.1 7.2 7.3 7.4 8.0 8.1 8.2 8.3 8.4; do echo Testing PHP $v...; if docker run --rm -v \"$PWD:/app\" -w /app php:${v}-cli sh -c \"find . -type f -name \\\"*.php\\\" -not -path \\\"*/vendor/*\\\" -print0 | xargs -0 -n1 php -l\" >/dev/null 2>&1; then echo Minimum supported PHP version: $v; exit 0; fi; done; echo No supported PHP version detected; exit 1'", |
There was a problem hiding this comment.
This currently excludes vendor content but the PHP compatibility issue was actually in a vendor dependency so this script wouldn't catch that.
Secondly, this relies on bash. Considering that we already have php-compatability checks, it should be enough to run those as part of the CI test matrix of all supported versions of PHP to catch any issues. It is important to run it also against the vendor contents which it doesn't do by default, if I'm not mistaken.
Finally, if we do keep it in as a convenience feature, let's reduce the range of PHP to 8.0 and above.
|
The detect-min-php is just for devs to double check, it's an overkill for CI, we can rely on the php version specified in composer for CI |
This whole breakage showed that we really can't rely on those 😅 For an infrastructure plugin like this we must handle our due diligence and verify those claims by running php compat on all source code and the dependencies. My suggestion is to not include |
|
We can add a comment maybe to not use it for CI. It's very useful as a dev tool as it gives the line numbers with errors for different php versions in verbose mode which is otherwise not possible without either using AI or actually running different php versions. |
| "require": { | ||
| "php": ">=8.0", | ||
| "fairpm/did-manager-wordpress": "0.0.4-a" | ||
| "fairpm/did-manager-wordpress": ">=0.0.5" |
There was a problem hiding this comment.
Let's use the ^x.x.x syntax here to ensure it passes the composer validate --strict check.
Update fairpm/did-manager-wordpress version to ^0.0.7 Signed-off-by: Namith Jawahar <48271037+namithj@users.noreply.github.com>
|
Closing the PR in favour of a new one targeting did-manager 0.0.7 |
|
@namithj Looks like we'll need another bump for did-manager-wordpress after fairpm/did-manager-wordpress#14 is merged. |
2 participants
