search

tzunghaor / settings-bundle

Symfony bundle for storing settings in database with editor GUI
MIT License
5 stars 1 forks source link

Allow Symfony 6, require php 8 #2

Closed tacman closed 1 year ago

tacman commented 2 years ago

What do you think of dropping the minimum version of PHP to 8.0 ?

Also, allowing for Symfony 6.

tzunghaor commented 2 years ago

Indeed the next task on my list is to test it with PHP 8 and Symfony 6, though I still would like to keep PHP 7 and Symfony 5 support for some time.

tacman commented 2 years ago

I'll submit a PR, it's almost working now for me (except for the form submit issue).

On Tue, Aug 23, 2022 at 2:37 PM Andras @.***> wrote:

Indeed the next task on my list is to test it with PHP 8 and Symfony 6, though I still would like to keep PHP 7 and Symfony 5 support for some time.

— Reply to this email directly, view it on GitHub https://github.com/tzunghaor/settings-bundle/issues/2#issuecomment-1224580233, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEXIQMHALS4JJ5TB7X7GCDV2ULADANCNFSM57LICI3A . You are receiving this because you authored the thread.Message ID: @.***>

tacman commented 2 years ago

What do you think of reorganizing the code to be more compliant with the recommended best practices of a Symfony bundle? In particular, moving the php code to /src and setting the PSR-4 namespace accordingly.

I'm trying to use rector to do some updates, and it'd be a bit easier if the source were in /src.

Also, I'm running into in error when running tests:

PHP Fatal error: Cannot use 'mixed' as class name as it is reserved in /home/tac/survos/bundles/settings-bundle/vendor/phpunit/phpunit/src/Framework/MockObject/MockClass.php(45) : eval()'d code on line 3

Nonetheless, I'll submit a very basic PR for Symfony 6 that at least works in my project.

tzunghaor commented 2 years ago

So Symfony changed recommended bundle structure for 6.0. I'll check it, but I won't have time for this before the weekend.

tacman commented 2 years ago

Yes. It's a bit cleaner now: https://symfony.com/doc/current/bundles.html#bundle-directory-structure

tacman commented 2 years ago

I did the refactoring in my branch, it should be in that same PR.

The main reason I wanted to do that was to run phpstan, which still shows a few errors, in particular:

Property Tzunghaor\SettingsBundle\Service\SettingsEditorService::$authorizationChecker has unknown class Tzunghaor\SettingsBundle\Service\AuthorizationChecker as its type.
tzunghaor commented 2 years ago

Hi again, @tacman - I merged your PR into php8 branch, fixed paths for test project and fixed phpstan errors. Could you composer require tzunghaor/settings-bundle:dev-php8 in your project and see if it works? I would then merge it to master and create a new release.

tacman commented 2 years ago

I'm traveling now, but should be able to look at it tomorrow.

Can you document how to use scope (user, project, whatever), or put something in the demo app? That's been my hold-up for using it in my projects. Thanks.

On Sun, Sep 4, 2022 at 10:03 AM Andras @.***> wrote:

Hi again, @tacman https://github.com/tacman - I merged your PR into php8 branch, fixed paths for test project and fixed phpstan errors. Could you composer require tzunghaor/settings-bundle:dev-php8 in your project and see if it works? I would then merge it to master and create a new release.

— Reply to this email directly, view it on GitHub https://github.com/tzunghaor/settings-bundle/issues/2#issuecomment-1236348619, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEXIQMINB6D7I3QBEW6BDLV4ST27ANCNFSM57LICI3A . You are receiving this because you were mentioned.Message ID: @.***>

tzunghaor commented 2 years ago

Created a PR in settings-demo-bundle with user and user-project scoped examples: https://github.com/tacman/settings-bundle-demo/pull/1

tzunghaor commented 1 year ago

It took long, but I finally got the energy to setup tests for multiple PHP / Symfony versions, and made a new release 0.13 including your changes. Thank you for the contribution.