Skip to content

Issue #124 : Symfony6 feature#126

Open
Alexandre-T wants to merge 8 commits intorvanlaak:3.xfrom
Alexandre-T:symfony6-feature
Open

Issue #124 : Symfony6 feature#126
Alexandre-T wants to merge 8 commits intorvanlaak:3.xfrom
Alexandre-T:symfony6-feature

Conversation

@Alexandre-T
Copy link
Contributor

@Alexandre-T Alexandre-T commented Aug 4, 2022

Do NOT merge this PR into master!

@rvanlaak , I'm trying to prepare a new version of your awesome bundle. A version only for symfony 5.4.|^6.0 and Php 7.4.|^8.0 .

The bundle now works fine with my Symfony 6.1 application, but I still have problem with the functional test. I already tried to upgrade from nyholm/symfony-bundle-test:1 to nyholm/symfony-bundle-test:2 . But I'm not able to detect the services provided by your bundle.

Any help is welcomed!

Symfony 3.4, 4.4, 5.3- removed
* Upgrading nyholm/symfony-bundle-test
* Fixing PhpStan configuration
@Alexandre-T Alexandre-T changed the title Symfony6 feature Symfony6 feature for #124 Aug 4, 2022
@Alexandre-T Alexandre-T changed the title Symfony6 feature for #124 Issue #124 : Symfony6 feature Aug 4, 2022
@rvanlaak
Copy link
Owner

rvanlaak commented Aug 4, 2022

Hi @Alexandre-T thanks for your work on this. As the old version of this bundle works fine for lower versions, I think it's fine to release a v4 (or v3, as only betas were released?) that bumps all requirements to package versions that do not require us to keep the BC layers.

So, what about bumping to PHP >=8.0 and Symfony >= 5.4 ?

@Alexandre-T
Copy link
Contributor Author

I do agree. I'm already removing PHP7.4 and fixing some potential issues with PhpStan

Removing psr/cache:2 and less
@Alexandre-T
Copy link
Contributor Author

I'm currently fixing the errors detected by php-stan.

As we prepare a new version, what about to fix parameters and returns with no typehint specified?

The most impacted interface is the serializer itself.

interface SerializerInterface
{
    public function serialize(mixed $data): string;

    public function unserialize(string $serialized): mixed;
}

But I guess impacts won't be so important, because this bundle already provides two internal serializers.

Do you agree with such changes?

(Sorry, I'm not so fluent in English.)

@rvanlaak
Copy link
Owner

Hi @Alexandre-T , yes feel free to improve the internal interface, as this will be a major bump for everything anyways..

$cachedSettingsManager->expects($this->at(1))
->method('invalidateCache')
->with($this->equalTo(null), $this->equalTo($owner))
->withConsecutive(

Choose a reason for hiding this comment

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

->at() and ->withConsecutive() are both not supported in PhpUnit 10 ;(
Maybe there is a trait to solve this. Or later in a seperate PR

@Chris53897
Copy link

I made a PR that could fix the remaining test problems

@rvanlaak
Copy link
Owner

rvanlaak commented Feb 7, 2023

@Chris53897 can you also file a PR for this repo?

@Chris53897 Chris53897 mentioned this pull request Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants