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

[PHP8.1] Added some fixes for PHP 8.1 #1843

Merged
merged 5 commits into from
Dec 14, 2022
Merged

[PHP8.1] Added some fixes for PHP 8.1 #1843

merged 5 commits into from
Dec 14, 2022

Conversation

AndreasK79
Copy link
Contributor

Credits to @mattmelling. I found these on his fork.

Everything seems to work ok on my local test.

@phl0 mind testing on 8.1?

@mattmelling
Copy link
Contributor

Sorry, should have submitted these changes upstream to you guys. Policy on changes to CodeIgniter is clearer now so will bear that in mind.

My fork is to support NixOS which updates quite quickly and php7 support was dropped in last month's release, hence the hacky changes.

@AndreasK79
Copy link
Contributor Author

@mattmelling No worries. PR's are always welcome. Changes to CodeIgniter does make an upgrade more difficult, but as long as it fixes stuff...
Before the V2 release, I tested the latest CI 3 code base, but found that it didn't have the necessary fixes for 8.1, so I never bothered with an upgrade.

I did add an extra fix in profiler.php:496 compared to your fixes.

@phl0
Copy link
Contributor

phl0 commented Dec 14, 2022

@phl0 mind testing on 8.1?

Sure: https://github.com/AndreasK79/Cloudlog/pull/15

@phl0
Copy link
Contributor

phl0 commented Dec 14, 2022

Apart from that the big bunch of errors on top are now fixed. Previously it looked like this:

Screenshot from 2022-12-14 13-11-46

Those are gone now.

@AndreasK79
Copy link
Contributor Author

AndreasK79 commented Dec 14, 2022

Found a few places that doesn't work:

  1. Analytics/statistics (Years/mode/bands)
  2. CQ Map
  3. DXCC Map
  4. IOTA Map

They all fail with the same message:

<p>Severity: 8192</p>
<p>Message:  str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated</p>
<p>Filename: core/Output.php</p>
<p>Line Number: 457</p>

Edit: Fixed with replacing output.php from CI github repo.

@phl0
Copy link
Contributor

phl0 commented Dec 14, 2022

Jepp. Also found some places. Will fix finding by finding.

@phl0
Copy link
Contributor

phl0 commented Dec 14, 2022

Fix for the maps: https://github.com/AndreasK79/Cloudlog/pull/16

@AndreasK79
Copy link
Contributor Author

AndreasK79 commented Dec 14, 2022

Fix for the maps: AndreasK79#16

That is also one way to fix it. I fetched the updated file from the CI repo and pushed.

@AndreasK79
Copy link
Contributor Author

@phl0 did you find any more places that PHP8.1 didn't work?

@phl0
Copy link
Contributor

phl0 commented Dec 14, 2022

That is also one way to fix it. I fetched the updated file from the CI repo and pushed.

Ok I guess it also fixes the maps issue? If so I will just close my PR.

@AndreasK79
Copy link
Contributor Author

@phl0 yes, it does. I thought you were looking into other stuff, sorry.

@phl0
Copy link
Contributor

phl0 commented Dec 14, 2022

No worries. Still clicking myself through stuff.

@phl0
Copy link
Contributor

phl0 commented Dec 14, 2022

Finished. The rest seems to work ok. But no guarantee :D

@phl0
Copy link
Contributor

phl0 commented Dec 14, 2022

Just tested your 8.1 branch on my PHP 7.4.3 installation. No issues ;-)

@AndreasK79
Copy link
Contributor Author

Thanks @phl0.

@magicbug wanna test before merge?

@magicbug
Copy link
Owner

seems OK to go into dev branch

@AndreasK79 AndreasK79 merged commit 5c565bf into magicbug:dev Dec 14, 2022
@AndreasK79 AndreasK79 deleted the php8.1_fixes branch January 27, 2023 09:22
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.

4 participants