heiseonline / shariff-backend-php

👮 PHP backend for Shariff. Shariff enables website users to share their favorite content without compromising their privacy.
http://ct.de/-2467514
133 stars 44 forks source link

feat: Support for PHP-8 #181

Closed erkenes closed 12 months ago

erkenes commented 2 years ago

Adjust the classes and dependencies to support PHP-8.1

Resolves: #170 Resolves: #175

richard67 commented 1 year ago

I have merged this PR into a branch on my fork and have tested it and can confirm that it works.

So this PR is ok, maybe except of some code style changes with which I'm not happy, so I've reverted them in my fork.

liayn commented 1 year ago

@richard67 Thanks for the info and your work.

Code-Style-wise I highly recommend sticking to PSR-12. https://www.php-fig.org/psr/psr-12/

Please let me know, when you merged your branch into your master branch. I will then finally take your sources for Shariff and this backend for the official TYPO3 integration. Many people will be happy about this major move forward!

richard67 commented 1 year ago

Code-Style-wise I highly recommend sticking to PSR-12. https://www.php-fig.org/psr/psr-12/

@liayn I know it, we use it for Joomla, of which I'm a maintainer.

I will then finally take your sources for Shariff

You mean also https://github.com/richard67/shariff-plus for the frontend? Or only my fork of the backend?

My shariff-plus has - in addition to shariff - a "Like" button for Facebook, but I would not recommend to use that button in EU because it doesn't work since Facebook have hardened their data protection. Besides that button, the difference to shariff is that shariff-plus it up to date with the latest changes merged in or proposed for shariff. I have created a new release 2.3.0 yesterday of that.

My fork of the backend I will maybe make ready tomorrow or Sunday and do a release (or pre-release or draft release, as long as I'm not sure if this repo here is really abandoned or not).

If you after that decide it to use for the official TYPO3 integration, give me a note so I can properly mention it in my README.md.

But to be honest, I'm not really sure if I want to take that responsibility. I'm doing this only as hobby, and I cannot promise to maintain all forever. But at least as long as I use it for my website I will keep it alive.

liayn commented 1 year ago

Yes, we will use frontend and backend.

My shariff-plus has - in addition to shariff - a "Like" button for Facebook, but I would not recommend to use that button in EU because it doesn't work since Facebook have hardened their data protection....

That's fully up to the integrator anyways, which button they wanna use.

If you after that decide it to use for the official TYPO3 integration, give me a note so I can properly mention it in my README.md.

Our TYPO3 extension: https://extensions.typo3.org/extension/rx_shariff

Sure, I'll ping you.

But to be honest, I'm not really sure if I want to take that responsibility. I'm doing this only as hobby, and I cannot promise to maintain all forever. But at least as long as I use it for my website I will keep it alive.

Don't worry about this. We lived with the non-maintained original way too long actually. Any improvement is an improvement. (And we maintain our extension also for free.)

richard67 commented 12 months ago

@liayn I would like to clarify a few questions, e.g. which minimum PHP version we should require, or in other words if we still should support 7.4 and 8.0, but I think we should not pollute this PR here with unrelated discussions. Maybe we continue with email in German language? You could send an email to admin<at>richard-fath<dot>de so I could reply.

richard67 commented 12 months ago

@liayn I've meanwhile released a new version 2.3.1 of my shariff-plus frontend, see https://github.com/richard67/shariff-plus/releases . Check also the change log for version 2.3.0 from 2 days ago to see what has changed, compared to heiseonline.

In my fork of the this repository here I've changed the default branch to 10.0-dev and just created a pre-release 10.0.0-alpha1. See https://github.com/richard67/shariff-backend-php/releases . It contains the changes from this PR here and other open PRs in this repository here.

Attention: That pre-release increases the PHP minimum version to 8.1, i.e. drops support for PHP 7.x and 8.0. All dependencies are updated to their latest version.

More attention: I am using the new releases on my website (URL is in my GitHub profile), and it seems to work. But I have tested only with PHP 8.1, not with 8.2 or 8.3, and I have tested the backend only with standard cache and client configurations.

The master branch I've kept in case if it needs to create also a version 9.1.0 which still supports PHP 7.4 and 8.0, but I'd prefer to avoid that.

Let me know your opinion by email to admin<at>richard-fath<dot>de.

Thanks in advance.

Update 2023-10-21 18:48 (UTC): I've just updated the zip and tar.gz packages of the 10.0.0-alpha1 release of my backend fork. The "shariff-backend-php-10.0.0-alpha1-dev" packages were wrong, the right ones are "shariff-backend-php-10.0.0-alpha1" (without "-dev" at the end of the base name).

digitalgopnik commented 12 months ago

Hello @erkenes! Thanks for your work on support php8 for our shariff-backend. We will merge this pull request, even though there are some changes which affect codestyle and are not PSR-12-confirm.

We see definitely the necessity of integrating phpcs-fixer or codesniffer! But as soon there aren't any checks or guidelines from our side, we won't stop the show. :slightly_smiling_face:

There will be a pull-request soon, which will integrate them.

@liayn @richard67 Thanks also for your work and interest in our shariff-backend

richard67 commented 12 months ago

@digitalgopnik Unfortunately this PR resulted in the composer.json and composer.lock not being consistent. When running "composer install" on a clean master branch, you get:

Warning: The lock file is not up to date with the latest changes in composer.json. You may be getting outdated dependencies. It is recommended that you run composer update or composer update <package name>.

When running "composer update" or when removing the composer.lock file, I get:

Your requirements could not be resolved to an installable set of packages.

Problem 1

  • guzzlehttp/guzzle[7.4.1, ..., 7.8.0] require php ^7.2.5 || ^8.0 -> your php version (7.2.0; overridden via config.platform, actual: 8.1.2) does not satisfy that requirement.
  • Root composer.json requires guzzlehttp/guzzle ^7.4.1 -> satisfiable by guzzlehttp/guzzle[7.4.1, ..., 7.8.0].

I think all this should not happen on a clean branch.

Shall I open a new issue for that?

I have only a dev environment with PHP 8.1 here, so I can't really fix that for older versions.

Or do you plan anyway to enlarge the version requirements?

digitalgopnik commented 12 months ago

@richard67 Thanks for your report! :pray: Somehow i disregarded that. Will take care of it, there's not needed another issue :slightly_smiling_face:

digitalgopnik commented 12 months ago

@richard67 Should be fixed now. Thanks again for your quick report!

richard67 commented 12 months ago

@richard67 Should be fixed now. Thanks again for your quick report!

@digitalgopnik Thanks. Last issue: The new release 10.0.0 is missing the 2 assets "shariff-backend-php.tar.gz" and "shariff-backend-php.zip".

richard67 commented 12 months ago

P.S.: The CHANGELOG.md should be updated for 10.0.0, too.

digitalgopnik commented 11 months ago

@richard67 Thanks for your attention! Should be correct now :slightly_smiling_face: