search

varspool / Wrench

A simple PHP WebSocket implementation for PHP 7.1
Do What The F*ck You Want To Public License
596 stars 210 forks source link

Fix php 7.4 compatibility problem and upgrade to phpunit 8.5 #130

Closed wells closed 4 years ago

wells commented 4 years ago

This is specifically to fix php 7.4 compatibility issues for jakubkulhan/chrome-devtools-protocol#24. Once the implode function args were flipped, I can confirm that v2.* of this package runs successfully in production on php 7.4.

In addition, I have upgraded the test environment to phpunit 8.5, given that phpunit 4.5 doesn't appear to function well on php 7.4.

nexen2 commented 4 years ago

Wells, can you mix your code with master branch and check is it working? Branch "2.0" is PHP 5.0 related, in time "master" is PHP 7.0+. Some of your fixes like "implode" deprecation problem can be found in "master" at moment. If your code will work [stable] with master branch, I can mix it in and mark with "3.0" tag. Finally.

wells commented 4 years ago

Hi @nexen2, I'll see what I can accomplish on master for you. Yeah, I can see that 2.0 has had php 5.* support.

I will need to make sure that the downstream chrome-devtools-protocol package can function okay with master / 3. of this package. Are there any breaking changes between 2. and 3.* to be aware of?

The majority of my changes in this pull were to get the 2.* branch working with phpunit 8.5 tests, which of course broke everything less than php 7.2.

nexen2 commented 4 years ago

As i know migration from v2 to v3 should be easy. Tests for 7.2+ only can add some complications to our life but I think it can be solved in some other way.

пт, 21 февр. 2020 г., 19:59 Brian Wells notifications@github.com:

Hi @nexen2 https://github.com/nexen2, I'll see what I can accomplish on master for you. Yeah, I can see that 2.0 has had php 5.* support.

I will need to make sure that the downstream chrome-devtools-protocol package can function okay with master / 3. of this package. Are there any breaking changes between 2. and 3.* to be aware of?

The majority of my changes in this pull were to get the 2.* branch working with phpunit 8.5 tests, which of course broke everything less than php 7.2.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/varspool/Wrench/pull/130?email_source=notifications&email_token=ACX37MURNWSPZB736S53U5LREAJA5A5CNFSM4KYYNGA2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMTRSFQ#issuecomment-589764886, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACX37MVHKA6F4WNR3OO35DTREAJA5ANCNFSM4KYYNGAQ .

wells commented 4 years ago

@nexen2 I will test drive dev-master of this package from a branch I have created for jakubkulhan/chrome-devtools-protocol

Is dev-master ready to be tagged 3.0 or do you still have a roadmap of work until a release is ready?

Given dev-master already has the implode() arguments swapped around for php 7.4 support, I do not have any additional changes to contribute to dev-master.

wells commented 4 years ago

@nexen2 I have successfully tested dev-master of wrench and was able to use it successfully with jakubkulhan/chrome-devtools-protocol.

Once you tag 3.0 I can push a pull request to the upstream package to depend on the changes from master.

damnedest commented 4 years ago

Any updates on moving to PHP 7.4?

damnedest commented 4 years ago

@nexen2 @dominics can you merge PR and update version?

wells commented 4 years ago

@nexen2 could you merge and tag this pull request as v3.0 and have the upcoming v3.0 tagged as v4.0? Without this tagged as a stable release, I am forced to set "minimum-stability": "dev" on my application's composer.json. It's starting to cause havoc.

iwiznia commented 4 years ago

Hi! Arrived here through this. I'm using this lib and experiencing the issue when trying to update to php 7.4, can this get merged and released or is there anything left to do? I tested this in my codebase and it works properly.