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

feat: drop support for php 8.0 #44607

Merged
merged 5 commits into from
Apr 8, 2024
Merged

feat: drop support for php 8.0 #44607

merged 5 commits into from
Apr 8, 2024

Conversation

st3iny
Copy link
Member

@st3iny st3iny commented Apr 2, 2024 •
edited
Loading

Summary

PHP 8.0 has been EOL for a long time now. Even debian stable is on PHP 8.21.

This change should also be reflected in the documentation and workflow template repository.

Checklist

Footnotes

  1. https://packages.debian.org/bookworm/php ↩

@st3iny st3iny added the 3. to review Waiting for reviews label Apr 2, 2024
@st3iny st3iny added this to the Nextcloud 30 milestone Apr 2, 2024
@st3iny st3iny self-assigned this Apr 2, 2024
nickvergessen
nickvergessen approved these changes Apr 2, 2024
View reviewed changes
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

The missing platform was not Debian, but SLES.
After some more email communication they made it temporarily available on packagehub until 15 service pack 6 will ship it later this year:
https://packagehub.suse.com/packages/php81/

So this is okay now

come-nc
come-nc reviewed Apr 2, 2024
View reviewed changes
lib/versioncheck.php
Comment on lines +28 to +29
// Show warning if a PHP version below 8.1 is used,
if (PHP_VERSION_ID < 80100) {
Copy link
Contributor

Choose a reason for hiding this comment

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

PhpOutdated setup check also needs to be adapted or removed.
I would say we can make it warn when PHP is <8.2, since 8.1 will be EOL soon after next release?

Copy link
Member Author

Choose a reason for hiding this comment

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

I adjusted the check.

You are currently running PHP 8.1.26. PHP 8.1 is now deprecated in Nextcloud 30. Nextcloud 31 may require at least PHP 8.2. Please upgrade to one of the officially supported PHP versions provided by the PHP Group as soon as possible. For more details see the documentation ↗.

@st3iny st3iny added the pending documentation This pull request needs an associated documentation update label Apr 2, 2024
@susnux
Copy link
Contributor

susnux commented Apr 2, 2024

Looks like psalm reports new errors, that should either be fixed or added to baseline

@come-nc
Copy link
Contributor

come-nc commented Apr 2, 2024

Also drone images use 8.0, so we need to upgrade or migrate to github CI

@susnux susnux mentioned this pull request Apr 2, 2024
Merged
@st3iny
Copy link
Member Author

st3iny commented Apr 2, 2024

Why does Psalm check files inside 3rdparty?

3rdparty/symfony/routing/Route.php:109:27: MethodSignatureMustProvideReturnType: Method Symfony\Component\Routing\Route::unserialize must have a return type signature!

@come-nc
Copy link
Contributor

come-nc commented Apr 4, 2024

Why does Psalm check files inside 3rdparty?

3rdparty/symfony/routing/Route.php:109:27: MethodSignatureMustProvideReturnType: Method Symfony\Component\Routing\Route::unserialize must have a return type signature!

I think it always did, because we need to include 3rdparty files for class references, and we never added anything to ignore errors in there.
Add this one to the baseline.

@come-nc come-nc mentioned this pull request Apr 4, 2024
Merged
7 tasks
@come-nc
Copy link
Contributor

come-nc commented Apr 8, 2024

Conflict because of #44663 fixing some of the same psalm errors xD

st3iny and others added 4 commits April 8, 2024 11:28
@st3iny
feat: drop support for php 8.0
9ef70f0 
Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
@come-nc @st3iny
fix: Fix new psalm errors from update
ab6afe0 
Not sure about the SimpleContainer modification, let’s see what CI says
 about that.

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@st3iny
ci(drone): use *-php8.3 images
0e483a0 
Ref nextcloud/docker-ci#634

Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
@come-nc @st3iny
chore: Add 3rdparty error to baseline. Also removed fixed errors.
b70180a 
The CDATA stuff comes from a psalm new version, nothing to worry about,
 both syntax are equivalent.

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@st3iny st3iny force-pushed the feat/drop-php-8.0 branch 2 times, most recently from 464e38c to 4fd9549 Compare April 8, 2024 09:31
@AndyScherzinger @nickvergessen
chore(drone): Update drone file signature
e0b5366 
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
@st3iny st3iny merged commit e108534 into master Apr 8, 2024
161 checks passed
@st3iny st3iny deleted the feat/drop-php-8.0 branch April 8, 2024 10:05
@susnux susnux mentioned this pull request Apr 8, 2024
Merged
@st3iny st3iny mentioned this pull request Apr 8, 2024
Merged
@ChristophWurst ChristophWurst mentioned this pull request Apr 9, 2024
Merged
@blizzz blizzz mentioned this pull request Jul 24, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nickvergessen nickvergessen nickvergessen approved these changes

@come-nc come-nc come-nc approved these changes

@susnux susnux susnux approved these changes

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst ChristophWurst is a code owner

@blizzz blizzz Awaiting requested review from blizzz

@miaulalala miaulalala Awaiting requested review from miaulalala miaulalala is a code owner

@tcitworld tcitworld Awaiting requested review from tcitworld tcitworld is a code owner

Assignees

@st3iny st3iny

Labels
3. to review Waiting for reviews pending documentation This pull request needs an associated documentation update
Projects
None yet
Milestone
Nextcloud 30
Development

Successfully merging this pull request may close these issues.
5 participants
@st3iny @susnux @come-nc @nickvergessen @AndyScherzinger