Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compatibility with php 8.4 #919

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

srjlewis
Copy link

@Geolim4 hope this helps

Also could you do a release of PHPSocialNetwork/couchbasev4-extension 😊

Deprecate implicitly nullable parameter types
https://wiki.php.net/rfc/deprecate-implicitly-nullable-types

@Geolim4
Copy link
Member

Geolim4 commented Oct 14, 2024

Thanks, I take a look tonight !

@Geolim4
Copy link
Member

Geolim4 commented Oct 14, 2024

I think I'm gonna throw travis Ci as it become more and more unstable over the years...

@srjlewis
Copy link
Author

Yep I dont blame you, I do think git actions dose a better job these days.

@Geolim4 Geolim4 changed the title Compatability with php 8.4 Compatibility with php 8.4 Oct 14, 2024
@srjlewis
Copy link
Author

@Geolim4

Could I give you a little nudge as a reminder on this pull 😁

Also do you mind if I push a release out for PHPSocialNetwork/couchbasev4-extension or would you prefur to do it?

@Geolim4
Copy link
Member

Geolim4 commented Oct 24, 2024

Can you add a commit to remove travis.yml please ?
As they don't support it, I have to get rid of them to keep only Github Actions.

I'll merge it tonight !

@srjlewis
Copy link
Author

Yes I have removed it for you.

@srjlewis
Copy link
Author

@Geolim4 It also looks like Scrutinizer is have trouble, it cant even compile PHP 8.0 and throwing a error.

@srjlewis
Copy link
Author

@Geolim4 I know you are a busy man, just a reminder nudge. 😊

@Geolim4
Copy link
Member

Geolim4 commented Nov 11, 2024

Can you take a look at why the tests fails on GetAllItems on 8.4 ?
I don't know if its related to this PR ?

@srjlewis
Copy link
Author

srjlewis commented Nov 12, 2024

Hi @Geolim4

TLDR: not related to this PR

The test that failed was on PHP 8.3.13

I was having a look yesterday when I see the tests fail. It looks like a redis, some thing has changed in the setup.
The only thing that looks to of changed could be the redis php extention https://pecl.php.net/package/redis

When I first subbmitted the PR all tests passed on my fork Attempt 1
When I see the tests fail yesterday I reran the same tests on my fork and it failed Attempt 2

I have been able to get the tests to pass by the following commit on a test branch srjlewis@61d2acf by removing redis from GetAllItems.test.php

I also have it passing with PHP 8.4 with the above fix https://github.com/srjlewis/phpfastcache/actions/runs/11795587335

I have also noticed that the tests are still running against redis 5, would it be a better idea to move redis to an extention and then also test against redis 6 and 7, just a idea so dont shot me 🤪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants