Fix broken assertion; minor other fixes #233
Conversation
| @@ -223,7 +223,7 @@ private static function getMarshaler(string $name, callable $callback): ?Closure | |||
| CData $hint, | |||
| ?CData $data | |||
| ) use (&$callback): void { | |||
| assert($numberOfParams === 0); | |||
| assert($numberOfParams === 1); | |||
There was a problem hiding this comment.
This was causing a test to fail.
I saw that the other asserts checked the param count based on the commented signature - 1, so it looks like this one should have been 1.
As to why it doesn't break in git actions, my guess is that they are running against an older version of libvips? (I noted the if (FFI::atLeast(8, 9)) { on line 217)
There was a problem hiding this comment.
As to why it doesn't break in git actions, my guess is that they are running against an older version of libvips? (I noted the if (FFI::atLeast(8, 9)) { on line 217)
Actually, that doesn't seem to be the case. A slightly older successful build used libvips 8.12: https://github.com/libvips/php-vips/actions/runs/7757736861/job/21160084456#step:4:121
Maybe PHP is running in production mode in the GitHub actions, which disables assertions? If that's the case, @jcupitt shouldn't we enable enable them?
| self::$library_major == $x && self::$library_minor == $y && self::$library_micro >= $z; | ||
| return self::$library_major > $x | ||
| || (self::$library_major === $x && self::$library_minor > $y) | ||
| || (self::$library_major === $x && self::$library_minor === $y && self::$library_micro >= $z); |
There was a problem hiding this comment.
I don't think this fixes (or breaks) anything, but at least it should be clearer now.
uuf6429
force-pushed
the
fix-broken-assertion
branch
from
46100f7 to
e4f45d5
Compare
Oops, I just realised this change is basically the same as #207.
This could be merged sooner if the other PR needs more time to be finished (otherwise you can also close this PR, as you wish).