prooph / standard-projections

Standard projections to use with Prooph EventStore
http://getprooph.org/
BSD 3-Clause "New" or "Revised" License
15 stars 4 forks source link

php8 compatibility #15

Closed basz closed 2 years ago

basz commented 2 years ago

I'l start with this one

After these changes a test fails. The order in which events are returned seem to change, but looking at it that seems correct.

When I pin prooph/event-store to lower then 7.5.3 the test passes again.

Either the test assumed a wrong order or prooph/event-store introduced a bug in 7.5.3

@prolic could you verify the order of the events in this failing assertion?

1) ProophTest\StandardProjections\AllStreamProjectionRunnerTest::it_emits_events_for_all_streams
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => '3'
+    0 => 'a'
basz commented 2 years ago

phpunit 9.3.0 is the first version to support php8, but does not supoort php7.1 and 7.2

I am okay with dropping support lower then 7.4 (like event-store does: "^7.4|^8.0"). What do you think?

cc @fritz-gerneth

fritz-gerneth commented 2 years ago

Personally I follow the policy to not support PHP versions not receiving security fixes.

codeliner commented 2 years ago

@basz regarding the failing test: I think the assertion needs to be changed. You introduced a new behavior in 7.5.3 that changes the order of events ;), see https://github.com/prooph/pdo-event-store/issues/201

The expected behavior. It should process events in order across streams.

basz commented 2 years ago

lol. i changed that.

I also dropped php81 for now as that gave some issues with ramsey/uuid when installing with --prefer-lowest. I think that may be better handled in a separate PR

basz commented 2 years ago

travis reports green