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.

Sign up for GitHub

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

Jump to bottom

Attempt to get working under PHP 8.0 and 8.1 #161

Merged
merged 2 commits into from
Dec 15, 2022
Merged

Attempt to get working under PHP 8.0 and 8.1 #161

merged 2 commits into from
Dec 15, 2022

Conversation

nigelgbanks
Copy link
Member

@nigelgbanks nigelgbanks commented Aug 4, 2022 •
edited
Loading

Upgrade symphony dependencies to ^4.4 to match up with Drupal 9.

Gonna test with all the other services at once, will post my results.

PHP is stopping security updates for 7.4 in November.

https://www.php.net/supported-versions.php

@nigelgbanks nigelgbanks force-pushed the upgrade-php branch 3 times, most recently from fb2c298 to e4e3421 Compare August 4, 2022 16:13
@nigelgbanks nigelgbanks changed the title Attempt to get working under PHP 8.1 Attempt to get working under PHP 8.0 and 8.1 Aug 4, 2022
@codecov
Copy link

codecov bot commented Aug 4, 2022 •
edited
Loading

Codecov Report

Base: 75.55% // Head: 75.34% // Decreases project coverage by -0.20% ⚠️

Coverage data is based on head (3dee7c6) compared to base (4dca22a).
Patch has no changes to coverable lines.

Additional details and impacted files 
@@             Coverage Diff              @@
##                3.x     #161      +/-   ##
============================================
- Coverage     75.55%   75.34%   -0.21%     
- Complexity      159      160       +1     
============================================
  Files             6        6              
  Lines           589      572      -17     
============================================
- Hits            445      431      -14     
+ Misses          144      141       -3
Impacted Files Coverage Δ
...ild_dir/Recast/src/Controller/RecastController.php 52.89% <0.00%> (-0.73%) ⬇️
...d_dir/Houdini/src/Controller/HoudiniController.php 98.21% <0.00%> (-0.18%) ⬇️
...r/Hypercube/src/Controller/HypercubeController.php 100.00% <0.00%> (ø)
build_dir/Milliner/src/Service/MillinerService.php 71.80% <0.00%> (+0.56%) ⬆️
...d_dir/Homarus/src/Controller/HomarusController.php 85.71% <0.00%> (+1.40%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@nigelgbanks nigelgbanks force-pushed the upgrade-php branch 3 times, most recently from 666a386 to bbdcfd2 Compare August 4, 2022 16:48
@whikloj
Copy link
Member

whikloj commented Aug 4, 2022

Hey @nigelgbanks you are welcome to work on this, but I wanted to note that #133 is using Symfony 4.4 for all the microservices. It doesn't currently test PHP 8 but it might be a lower bar.

@nigelgbanks
Copy link
Member Author

Hi @whikloj, thanks for pointing that out, at the moment I'm testing infrastructure changes that require a move to PHP 8.1 I'll probably stick to working against 2.x until 3.x is officially released.

@whikloj
Copy link
Member

whikloj commented Aug 4, 2022

I understand, I'm just not super confident on when that will happen.

@nigelgbanks
Copy link
Member Author

Just a heads up for 8.1 tests are failing due to guzzle https://github.com/guzzle/psr7/blob/8f0a09ce19adcf28f25ff9d90e7329488501b363/src/StreamWrapper.php#L47 is passing a NULL where it should be FALSE as PHP 8.1 has deprecated this. I'll need to get that sorted first. I tested with patching locally but I don't think we want to maintain a patch, better to fix it at the source.

@nigelgbanks nigelgbanks changed the base branch from 2.x to 3.x November 6, 2022 20:34
@nigelgbanks
Copy link
Member Author

So looking in the previous comment it would force me to update chullo which would affect crayfish-commons which would affect the islandora module, this would force users onto Drupal 9.4 and later. So I'll just patch it.

@nigelgbanks
Copy link
Member Author

Can't easily patch it I'll just change the setting to ignore deprecations in the unit tests.

@nigelgbanks nigelgbanks marked this pull request as ready for review November 6, 2022 21:24
@nigelgbanks
Copy link
Member Author

@whikloj it's finally ready for review 🙃

@rosiel rosiel requested a review from alxp November 9, 2022 18:39
whikloj
whikloj reviewed Nov 14, 2022
View reviewed changes
.scripts/tester Outdated Show resolved Hide resolved
@nigelgbanks
Attempt to get working under PHP 8.1
2ce7e85 
- Removed lock file
- Updated test matrix
- Fixed tests to work on php 7.4, 8.0, 8.1

For reasons behind removing the lock file see:
https://islandora.slack.com/archives/CM5PPAV28/p1659631615201049
Islandora/documentation#1908 (Removal approved)

Instead we'll provide lock files in isle and ansible deployments.

Additionally moving to drop tests for 7.3 as it is no longer supported
by Drupal 9.4 and up.
@nigelgbanks
Removed debug code
3dee7c6
@nigelgbanks nigelgbanks requested a review from whikloj December 13, 2022 10:03
@whikloj
Copy link
Member

whikloj commented Dec 13, 2022

I'm sorry @nigelgbanks, I'm trying to build a vagrant box with 8.0 to test this right now.

whikloj
whikloj approved these changes Dec 15, 2022
View reviewed changes
Copy link
Member

@whikloj whikloj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with PHP 8.0 and 8.1 👍

@whikloj whikloj merged commit d4bc375 into Islandora:3.x Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whikloj whikloj whikloj approved these changes

@alxp alxp Awaiting requested review from alxp

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nigelgbanks @whikloj