laminas / laminas-mvc-plugin-fileprg

Post/Redirect/Get plugin with file upload handling for laminas-mvc controllers
https://docs.laminas.dev/laminas-mvc-plugin-fileprg/
BSD 3-Clause "New" or "Revised" License
9 stars 8 forks source link

Drop `laminas/laminas-zendframework-bridge` and `zendframework/*` compatibility #17

Closed PowerKiKi closed 2 years ago

PowerKiKi commented 2 years ago
Q A
Documentation no
Bugfix yes
BC Break no
New Feature no
RFC no
QA no

Description

Increase performance by removing a compatibility layer while not introducing breaking changes.

This follow the process described in details in:

https://github.com/laminas/technical-steering-committee/blob/main/meetings/minutes/2021-08-02-TSC-Minutes.md#remove-laminaslaminas-zendframework-bridge-dependency-from-our-packages

PowerKiKi commented 2 years ago

I think the failure is not related to this PR.

I found out that PHPUnit runs a test in a separate process and try to serialize things that are surprisingly not serializable. Specifically this will fail:

<?php

use Laminas\Session\Container;

require './vendor/autoload.php';

$serialized = serialize(new Container());
$unserialized = unserialize($serialized);

I am not quite sure whether the bug is in Laminas\Session, because it is supposed to be serializable, or in this package because our test is not written properly. Any hint would be welcome.

Ocramius commented 2 years ago

Sorry, my comment was rushed: yes, the failures are not related to this patch, but we need to solve them before proceeding with merges in general.

PowerKiKi commented 2 years ago

Thank you for the precision. I'm afraid I'll have to give up on this PR, since it's a package I never used myself and cannot reasonably spend more time on this.

Feel free to reject this PR, or complete it, whichever seems the most appropriate to you.

weierophinney commented 2 years ago

Closing in favor of #18, as we've found fixes for the various testing failures noted here while working on it, and it also removes the bridge requirement.