[Symfony 2.7] Fix deprecation warnings #1221

andrerom · 2015-04-08T18:35:32Z

Factories: https://jira.ez.no/browse/EZP-24460
SecurityContext: https://jira.ez.no/browse/EZP-24461
Yaml::parse(): https://jira.ez.no/browse/EZP-24464
Synchronized SiteAccess: https://jira.ez.no/browse/EZP-24459
Request injection: https://jira.ez.no/browse/EZP-24457
Require Symfony 2.7: https://jira.ez.no/browse/EZP-24465

Candidates for backports to 5.4

Fix EZP-24460: Fix service factories deprecation warnings 7263ef4
Fix EZP-24464: Fix Yaml::parse() usage 0db5901
EZP-24458: Silent deprecation warnings on syncronized FakeServices 7473570
Fix EZP-24465: Bump Symfony requirement to v2.7 da19b18
EZP-24459: Manually set ezpublish.siteaccess service synchronized 36fb72c

Follow-ups

Inject RequestStack instead of Request => EZP-24457
Refactor Dynamic services => EZP-24458
Enable DebugBundle in test+dev => Fix EZP-24465: Bump Symfony requirement to v2.7 ezplatform#21

andrerom · 2015-04-11T09:16:09Z

eZ/Bundle/EzPublishRestBundle/Resources/config/services.yml

A bit unsure what factory w/o method means, so is this by any chance a left over?

Weird...
Ping @bdunogier

andrerom · 2015-04-11T18:16:18Z

eZ/Bundle/EzPublishCoreBundle/DependencyInjection/Compiler/ConfigResolverParameterPass.php

I'm not to familiar with these Fake services, so unsure if it is another/better way without big refactoring of this.

This will be a problem for us, for sure... I don't see how to workaround this future removal.
Ping @fabpot maybe?

lolautruche · 2015-04-13T09:34:45Z

eZ/Bundle/EzPublishCoreBundle/Resources/config/cache.yml

Should be:

ezpublish.cache_pool: class: %ezpublish.cache_pool.class% factory: ["@ezpublish.cache_pool.factory", getCachePool] arguments: [@ezpublish.config.resolver]

andrerom · 2015-06-03T10:47:18Z

It's not exactly the same thing. The RequestStack was introduced to solve the multiple requests problem (sub requests). This is not the case here.

@lolautruche hmm, it might actually be on second thought on this. In regards to the link issue today on linking from one site access to another there is basically a need to swap out current site access temporary right? So there is a possible need for stack here as well.

lolautruche · 2015-06-03T11:28:02Z

@andrerom The issue discussed with @larseirik has nothing to do with this. The SiteAccess is unique for requests/sub-requests. When doing a cross-siteaccess link, we don't swap out the current SiteAccess, we do a reverse lookup and complement generated link.
The only exception is content preview where we change the default scope, and there is a specific event-based mechanism for that. This is what needs to be fixed.

andrerom · 2015-06-03T11:34:16Z

ok, assumed the reverse lookup would have to swap siteaccess as well to make sure whole system generates the Url using the right settings, including taking right rootLocationId setting into account.

lolautruche · 2015-06-03T11:36:44Z

including taking right rootLocationId setting into account.

That might be the missing part of EZP-24470, but I still need to take some time to check what is wrong.

andrerom · 2015-06-03T11:37:12Z

ok

andrerom · 2015-06-03T11:39:24Z

Besides, as said on Skype, I'm ok with the proposed backports so +1 on those for master and 5.4.

As for my suggestion on splitting this branch, I was thinking you needed a few of the commits in master for the version bump, while others can mature a bit if needed.

lolautruche · 2015-06-03T11:40:51Z

I'll maybe just split out the SiteAccess commit, because it's incomplete and also has its separate issue.

andrerom · 2015-06-03T11:57:13Z

.travis.yml

As composer is bumped to use 2.7 by default we should rather change any use of SYMFONY_VERSION="~2.7" to SYMFONY_VERSION="~2.8@dev", no need to add additional job given 2.3 is out.

But isn't it what I did?

This PR has been rebased against master, so test with ~2.8@dev is in allow_failures. I was not adding a new test here. Or did I overlook something?

the diff above does add a new test in include section, while the change:

- env: TEST_CONFIG="phpunit.xml" SYMFONY_VERSION="~2.7@dev" + env: TEST_CONFIG="phpunit.xml" SYMFONY_VERSION="~2.7"

could have just been:

- env: TEST_CONFIG="phpunit.xml" SYMFONY_VERSION="~2.7@dev" + env: TEST_CONFIG="phpunit.xml" SYMFONY_VERSION="~2.8@dev"

hence making the change I comment on unneeded.

OK, but then we won't test with PHP 5.5 and Symfony 2.7... But I think that's OK?

Yes. We can't test everything on everything as it would lead to 30 or more jobs, so we spread the tests out. alternative is to not test 2.8.

andrerom · 2015-06-03T11:58:41Z

Ok, I'm +1 with SiteAccess separated, and nitpick above about on how we can test 2.8 w/o adding additional jobs covered.

(dynamic settings)

lolautruche · 2015-06-03T12:16:09Z

Removed SiteAccess change from the commit list. I'll do another PR for this.

andrerom · 2015-06-03T12:17:51Z

+1 if/when travis agrees :)

It's not possible to silent the deprecation warning from YAML.

lolautruche · 2015-06-03T15:32:11Z

Just added 36fb72c to fix tests.

andrerom · 2015-06-04T06:52:29Z

+1 ping @pspanja / @glye

lolautruche · 2015-06-04T07:04:02Z

Another round of review? I thought everyone was +1

[Symfony 2.7] Fix deprecation warnings

andrerom · 2015-06-04T08:44:17Z

missed the one from pspanja, but otherwise you have a broad definition of everyone ;)

lolautruche · 2015-06-04T08:49:53Z

everyone => Everyone involved in the discussion

> https://jira.ez.no/browse/EZP-24459 Follow-up of #1221. After investigation, it is actually not necessary to *fix* ContentPreview since the previewed SiteAccess is already correctly handled, as the content preview is rendered through a sub-request. In this regard, SiteAccess match done normally (not limited to master request).

andrerom changed the title ~~[Symfony 2.7] Fix use of deprecated features & dissalow tests to fail~~ [Symfony 2.7] Fix use of deprecated features Apr 8, 2015

andrerom changed the title ~~[Symfony 2.7] Fix use of deprecated features~~ [Symfony 2.7] Fix deprecation warnings Apr 8, 2015

andrerom force-pushed the sf27 branch 18 times, most recently from 7096233 to 10ec462 Compare April 10, 2015 23:28

andrerom reviewed Apr 11, 2015
View reviewed changes

andrerom force-pushed the sf27 branch from 10ec462 to 437f76a Compare April 11, 2015 12:20

andrerom reviewed Apr 11, 2015
View reviewed changes

andrerom changed the title ~~[Symfony 2.7] Fix deprecation warnings~~ [WIP][Symfony 2.7] Fix deprecation warnings Apr 13, 2015

lolautruche reviewed Apr 13, 2015
View reviewed changes

andrerom reviewed Jun 3, 2015
View reviewed changes

André R and others added 7 commits June 3, 2015 14:06

Fix EZP-24460: Fix service factories deprecation warnings

f43f83b

Fix EZP-24464: Fix Yaml::parse() usage

4f910b2

Fix EZP-24461: Fix SecurityContext deprecation warnings

4317a4d

EZP-24458: Silent deprecation warnings on syncronized FakeServices

019b5fb

(dynamic settings)

Added Symfony 2.8 to Travis testing Matrix

b318de0

Un-silence deprecation errors

067c9ea

Fix EZP-24465: Bump Symfony requirement to v2.7

81d3bdf

lolautruche force-pushed the sf27 branch from 83d3a29 to 81d3bdf Compare June 3, 2015 12:15

EZP-24459: Manually set ezpublish.siteaccess service synchronized

36fb72c

It's not possible to silent the deprecation warning from YAML.

lolautruche added a commit that referenced this pull request Jun 4, 2015

Merge pull request #1221 from ezsystems/sf27

a07cced

[Symfony 2.7] Fix deprecation warnings

lolautruche merged commit a07cced into master Jun 4, 2015

lolautruche deleted the sf27 branch June 4, 2015 08:40

lolautruche mentioned this pull request Jun 4, 2015

Fix EZP-24459: SiteAccess should not be a synchronized service any more #1280

Merged

[Symfony 2.7] Fix deprecation warnings #1221

[Symfony 2.7] Fix deprecation warnings #1221

Uh oh!

Conversation

andrerom commented Apr 8, 2015

Candidates for backports to 5.4

Follow-ups

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrerom commented Jun 3, 2015

Uh oh!

lolautruche commented Jun 3, 2015

Uh oh!

andrerom commented Jun 3, 2015

Uh oh!

lolautruche commented Jun 3, 2015

Uh oh!

andrerom commented Jun 3, 2015

Uh oh!

andrerom commented Jun 3, 2015

Uh oh!

lolautruche commented Jun 3, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrerom commented Jun 3, 2015

Uh oh!

lolautruche commented Jun 3, 2015

Uh oh!

andrerom commented Jun 3, 2015

Uh oh!

lolautruche commented Jun 3, 2015

Uh oh!

andrerom commented Jun 4, 2015

Uh oh!

lolautruche commented Jun 4, 2015

Uh oh!

andrerom commented Jun 4, 2015

Uh oh!

lolautruche commented Jun 4, 2015

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants