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.

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

Compatiblity with Symfony 4.3 #2784

Merged
merged 13 commits into from Jun 16, 2019
Merged

Conversation

dunglas
Copy link
Member

@dunglas dunglas commented May 10, 2019

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

@dunglas dunglas changed the base branch from master to 2.4 May 10, 2019 10:54
@dunglas dunglas changed the base branch from 2.4 to master May 10, 2019 10:54
@alanpoulain alanpoulain force-pushed the sf4.3-compat branch 4 times, most recently from 1aa8822 to 58ace00 Compare May 31, 2019 16:07
@alanpoulain alanpoulain marked this pull request as ready for review May 31, 2019 16:07
@alanpoulain alanpoulain force-pushed the sf4.3-compat branch 10 times, most recently from 4b2af33 to 5f58782 Compare June 1, 2019 18:01
@soyuka soyuka force-pushed the sf4.3-compat branch 2 times, most recently from f140dac to 04d6f0b Compare June 4, 2019 15:52
@soyuka soyuka force-pushed the sf4.3-compat branch 2 times, most recently from 315509b to ccfafca Compare June 14, 2019 14:55
@dunglas dunglas merged commit 6e2c268 into api-platform:master Jun 16, 2019
@dunglas dunglas deleted the sf4.3-compat branch June 16, 2019 06:37
@dunglas
Copy link
Member Author

dunglas commented Jun 16, 2019

Thank you very much @soyuka and @alanpoulain for the help!

@@ -30,8 +30,6 @@
/**
* Generic item normalizer.
*
* @final
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is right. The GraphQL ItemNormalizer should stop extending this class instead, but instead extend AbstractItemNormalizer.

@@ -96,10 +97,11 @@ protected function configureContainer(ContainerBuilder $c, LoaderInterface $load

$loader->load(__DIR__."/config/config_{$this->getEnvironment()}.yml");

$alg = class_exists(SodiumPasswordEncoder::class) && SodiumPasswordEncoder::isSupported() ? 'auto' : 'bcrypt';
Copy link
Contributor

Choose a reason for hiding this comment

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

Uhh... Symfony really screwed this one up. :x

Copy link
Contributor

Choose a reason for hiding this comment

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

It's not bcrypt if sodium isn't supported but the native encoder. I do not know if it's important here.
The bcrypt encoder is now deprecated

Copy link
Member Author

Choose a reason for hiding this comment

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

It's only for old versions so I think we don't care (this code will go be thrown away at some point).

@teohhanhui
Copy link
Contributor

It seems like this is not addressed? #2856

@soyuka
Copy link
Member

soyuka commented Jun 18, 2019

I'm not sure to understand how #2856 is an issue here...

@teohhanhui
Copy link
Contributor

@soyuka No, it's not an issue here. Just that I'd consider #2856 essential for Symfony 4.3 compatibility. Because it's not doing the right thing now.

@soyuka
Copy link
Member

soyuka commented Jun 21, 2019

True although I'm not sure to get what we need to do on our side.

@bastnic
Copy link
Contributor

bastnic commented Jun 26, 2019

Why only master and not 2.4 branch ? My tests are full of 59403x: Method "ApiPlatform\Core\Serializer\ItemNormalizer::createChildContext()" will have a third "?string $format" argument in version 5.0; not defining it is deprecated since Symfony 4.3.

@dunglas
Copy link
Member Author

dunglas commented Jun 26, 2019

Because Messenger. Messenger 4.3 introduces some BC breaks, that in cascade force us to add some BC breaks too. Also, to use Messenger 4.3 we had to bump the version of several other Symfony components. We then decided that it wasn't a good idea to introduce such changes in a patch release.
Anyway, we're working hard on finishing 2.5, a beta should be available soon.

@teohhanhui
Copy link
Contributor

We could certainly backport the other changes that are not related to Symfony Messenger.

@dunglas
Copy link
Member Author

dunglas commented Jun 26, 2019

Maybe some of them yes! Feel free to open a PR!

betd-sthibault pushed a commit to betd-sthibault/core that referenced this pull request Aug 7, 2019
* Compatiblity with Symfony 4.3

* Fix all broken unit tests

* Fix some deprecations

* Remove @Final from ItemNormalizer

* Fix Behat tests

* Use composition for the Exception Listener

* Fix default firewall logout_on_user_change deprecation

* Make sure that sodium is supported

* Fix composer and tests

* Fix PHPStan config and CS

* Remove deprecated option

* Fix MongoDB

* Fix rebase
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants