-
Notifications
You must be signed in to change notification settings - Fork 480
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
Upgrading Skeleton to PHP 8.4 #345
base: main
Are you sure you want to change the base?
Conversation
Hmm, but what if somebody wants to create a new app that should also be compatible with older PHP versions? |
This way, people like me, using PHP 8.1, will be required to use the old version because this version only permits use starting with PHP 8.3. Although all or most of the changes you made are compatible with PHP 8.0. |
I'd like to point out that you are on borrowed time with 8.1 as it went out of active support ~5 months ago Supported Versions. Looking at other popular frameworks they have also drop support for 8.1. So i can see the value in supporting PHP 8.2+ As for exisiting projects this doesn't change much as the chanages do not affect the underlying framework only new projects created via |
Well, that's stupid. 8.1 is the package manager version in Ubuntu Jammy (22.04). If there are newer Ubuntu versions, I can't upgrade because I'm using Linux Mint. It's bad that we have these great package managers but then have to wait to get newer versions – or use PPAs or build PHP or whatever. Well, Mint 22 should come out soon, with a new Ubuntu base. But 8.1 still gets security fixes so the skeleton...ah, readonly is new in 8.2 so we can't use it in 8.1. Well, okay, will hopefully hop on to 8.3 in next Ubuntu base. 😅 |
@carlosmintfan you might want to consider looking into docker if your developing locally, it’s very neat and decouples config from your development environment. |
Rework is needed to update this to PHP 8.4, hopefully I'll have time over the weekend. |
Bumping PHP versionn to 8.3
changing constructors to user new syntax where it makes sense
using match to get get error message
Adding union types for better type hinting
Updating the php version in README
Supporting all supported versions of PHP 8.2 and 8.3
af916e9
to
7f32837
Compare
Adding 8.2 to CI pipeline and being stricter with php versions in composer
Hi @odan Thanks for taking a look I've updated the PR, If there is anything else let me know and try to resolve it quickly. |
@@ -22,20 +22,20 @@ | |||
} | |||
], | |||
"require": { | |||
"php": "^7.4 || ^8.0", | |||
"php": "^8.2.0 || ^8.3.0 || ^8.4.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also covers higher minor versions then 8.4, which should not be the case. Instead define only a list of 8.2.* || 8.3.* || 8.4.*
.
@@ -8,7 +8,7 @@ This skeleton application was built for Composer. This makes setting up a new Sl | |||
|
|||
## Install the Application | |||
|
|||
Run this command from the directory in which you want to install your new Slim Framework application. You will require PHP 7.4 or newer. | |||
Run this command from the directory in which you want to install your new Slim Framework application. You will require PHP 8.3 or newer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minimum is 8.2
@@ -19,7 +20,7 @@ | |||
'logger' => [ | |||
'name' => 'slim-app', | |||
'path' => isset($_ENV['docker']) ? 'php://stdout' : __DIR__ . '/../logs/app.log', | |||
'level' => Logger::DEBUG, | |||
'level' => Level::fromName('DEBUG'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method call produces overhead. Instead we should use the generic PSR-4 LogLevel
instead. Example: \Psr\Log\LogLevel::DEBUG
.
{ | ||
$this->id = $id; | ||
public function __construct( | ||
private int|null $id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should keep property definitions separate from constructor parameter declarations. Mixing them makes the code less clean and harder to follow. Please re-apply this approach for all changed classes to keep things consistent and readable.
Updating the skeleton to user PHP 8.2+ and updating to some of the newer language features.
closes #344