[WIP] Symfony3 #331

hacfi · 2016-01-30T18:41:17Z

No description provided.

hacfi · 2016-01-30T18:52:01Z

@dbu Those are the changes I made to make it work with my current (not online yet) Symfony 3.0 project and no issues for the last 2 weeks. I’m gonna look into the deprecation calls and check if there are more changes required in this repository.

hacfi · 2016-01-30T18:58:51Z

Tests/Unit/Routing/DynamicRouterTest.php

@@ -89,6 +91,8 @@ public function testMatchRequest()
     */
    public function testMatchNoRequest()
    {
+        $this->markTestSkipped('Test for deprecated match() method');


See https://github.com/symfony-cmf/Routing/blob/master/DynamicRouter.php#L218

Should I just remove those tests?

Mark them as legacy and they won't be marked as error. You can do it by adding @group legacy to the method PHPdoc.

hacfi · 2016-01-30T19:29:39Z

Thanks for the feedback @wouterj. Updated the PR accordingly.

hacfi · 2016-01-30T22:44:06Z

I tried adding Symfony 3.0.* to the test matrix (https://travis-ci.org/symfony-cmf/RoutingBundle/jobs/105928993) but sonata-project/doctrine-phpcr-admin-bundle 1.1.0 requires symfony/framework-bundle ~2.3: https://github.com/sonata-project/SonataDoctrinePhpcrAdminBundle/blob/master/composer.json#L22 to that fails at the moment.

The COMPOSER_FLAGS="--prefer-lowest" build (https://travis-ci.org/symfony-cmf/RoutingBundle/jobs/105928991) fails with Argument 5 passed to Sonata\DoctrinePHPCRAdminBundle\Tree\PhpcrOdmTree::__construct() must be an instance of Symfony\Component\Templating\Helper\CoreAssetsHelper, instance of Symfony\Bundle\FrameworkBundle\Templating\Helper\AssetsHelper given which was fixed in sonata-project/SonataDoctrinePhpcrAdminBundle#337 but that would bump the requirements to version 1.2.3. Should I add this?

The DEPS=dev build keeps timing out at composer update --prefer-dist

wouterj · 2016-01-30T23:03:04Z

Thanks for putting so much time in this PR! It's great to see Symfony 3 coming closer each push.

I tried adding Symfony 3.0.* to the test matrix (https://travis-ci.org/symfony-cmf/RoutingBundle/jobs/105928993) but sonata-project/doctrine-phpcr-admin-bundle 1.1.0 requires symfony/framework-bundle ~2.3 [...] to that fails at the moment.

What you can do is using branch aliases to fool Composer that we're installing 2.8 and then excluding the web-test group (using --exclude-group) when running PHPunit with Symfony 3. It's a little hacky, but this allows us to see if everything works with Symfony 3 until SonataDoctrinePHPCR properly supports Symfony 3.

[...] which was fixed in sonata-project/SonataDoctrinePhpcrAdminBundle#337 but that would bump the requirements to version 1.2.3. Should I add this?

I see no problem bumping that requirement.

The DEPS=dev build keeps timing out at composer update --prefer-dist

Composer probably has a hard to resolving the versions. Let's narrow down the search by forcing Symfony 2.8-dev (by also setting SYMFONY_VERSION="2.8.*" for this Travis job).

wouterj · 2016-01-30T23:05:05Z

Btw, I think if we're going to release 2.0 of this bundle, I think we should just remove all BC layers and go with Symfony 2.8. We didn't discuss this much yet, but I suggest to not waste time creating BC layers to support 2.7.

hacfi · 2016-01-31T00:42:53Z

@wouterj Thanks for the advice! Unfortunately I’m not getting the branch alias working correctly:

With "symfony/framework-bundle": "v3.0.1 as v2.8.2" it still says

  Problem 1
    - sonata-project/doctrine-phpcr-admin-bundle 1.2.4 requires symfony/framework-bundle ~2.3 -> satisfiable by symfony/framework-bundle[v2.8.2].
    - sonata-project/doctrine-phpcr-admin-bundle 1.2.3 requires symfony/framework-bundle ~2.3 -> satisfiable by symfony/framework-bundle[v2.8.2].
    - sonata-project/doctrine-phpcr-admin-bundle 1.2.4 requires symfony/framework-bundle ~2.3 -> satisfiable by symfony/framework-bundle[v2.8.2].
    - Conclusion: don't install symfony/framework-bundle v2.8.2
    - Installation request for sonata-project/doctrine-phpcr-admin-bundle ^1.1,>=1.2.3 -> satisfiable by sonata-project/doctrine-phpcr-admin-bundle[1.2.3, 1.2.4].

Any idea how to get this running?

hacfi · 2016-01-31T02:11:39Z

Prefer lowest was failing because of sonata-project/SonataAdminBundle#2186 so I added sonata-project/admin-bundle >=2.2.12

wouterj · 2016-01-31T10:36:32Z

composer.json

-        "sonata-project/doctrine-phpcr-admin-bundle": "^1.1",
+        "sonata-project/admin-bundle": ">=2.2.12",
+        "sonata-project/core-bundle": ">=2.2.3",
+        "sonata-project/block-bundle": ">=2.2.8",


please use ~2.2.8 instead of >=. Sonata is very bad in semver (also applies to the 2 requirements on line 26 and 27)

Yep, I noticed! Should I change sonata-project/admin-bundle or sonata-project/core-bundle as well? I didn’t want to be to strict as I don’t know the recent development of the packages.

dbu · 2016-02-01T09:05:18Z

+1 for going with 2.7 or 2.8 and removing all BC hacks. also we can remove our own deprecated things but not necessarily in this PR.

hacfi · 2016-02-01T13:05:04Z

So anything else to do here to get this merged? I know the added version constraints are not great but unfortunately they are necessary.

dbu · 2016-02-01T13:13:21Z

composer.json

@@ -23,7 +23,10 @@
        "matthiasnoback/symfony-dependency-injection-test": "~0.6",
        "matthiasnoback/symfony-config-test": "^1.3.1",
        "phpunit/php-code-coverage": "@stable",
-        "sonata-project/doctrine-phpcr-admin-bundle": "^1.1",
+        "sonata-project/admin-bundle": ">=2.2.12",
+        "sonata-project/core-bundle": ">=2.2.3",


please use ^ and not >=, this would allow sonata 3 or 4 or any. (also the line above)

the caret operator means this patch version or any minor version higher than this.

dbu · 2016-02-01T13:15:17Z

coming good. we need to create the 1.4 branch before merging this, so that we still can release the 1.4 that supports symfony 2.3

this alone is no reason to jump the branch-alias to 2.0 so we can see when we need to do that.

hacfi · 2016-02-01T13:31:30Z

Updated. I just didn’t want to be too restrictive.

hacfi · 2016-02-16T17:55:47Z

@dbu Anything else to do here? I’d rather remove the Sf27 Compat stuff in a separate PR to not mix things up.

wouterj · 2016-02-16T22:38:47Z

.travis.yml

@@ -21,11 +21,9 @@ env:
 matrix:
  include:
    - php: 5.6
-      env: DEPS=dev
+      env: DEPS=dev SYMFONY_VERSION=2.8.*


if you change this value to 3.0.x-dev as 2.8.99, we should be able to test against Symfony 3 (I used the same successuflly line in another project)

Will try this again..thx!

Not working: https://travis-ci.org/symfony-cmf/RoutingBundle/jobs/109766483 :(

wouterj · 2016-02-16T22:40:33Z

@hacfi I think this PR is ready. However, we're focusing on the 1.4 stable release first. After that's done (expect end February at the very last), we start bumping to 2.0 and merging the 2.0 PRs.

hacfi · 2016-02-16T23:21:07Z

That sounds great!

Maybe it would be helpful for others to pull my branch into this repo so they can also use it as dev-symfony3 - just a suggestion :).

dbu · 2016-02-17T08:15:07Z

yep, exactly, i want to wait until we have 1.4 released and start the 1.4 branch, then switch master to 2.0 and merge this.

good idea with the branch. that would also allow to do that follow up PR to remove BC hacks - i agree its better to do that separately. i just created a symfony3 branch from master. can you close this PR and open a new one against the symfony3 branch? then we get that merged asap and can continue further steps in separate PRs.

…Sonata dependencies, use parentDocument in Admin classes, update route_basepaths in tests

hacfi · 2016-02-17T09:22:48Z

Done: #332

lsmith77 added the wip/poc label Jan 30, 2016

hacfi reviewed Jan 30, 2016
View reviewed changes

hacfi mentioned this pull request Jan 30, 2016

[WIP] Symfony 3 #323

Closed

wouterj reviewed Jan 31, 2016
View reviewed changes

dbu reviewed Feb 1, 2016
View reviewed changes

wouterj reviewed Feb 16, 2016
View reviewed changes

Allow Symfony ~3.0, bump Symfony dependency to ~2.7, specify correct …

13e401f

…Sonata dependencies, use parentDocument in Admin classes, update route_basepaths in tests

hacfi mentioned this pull request Feb 17, 2016

Symfony3 compatibility #332

Merged

hacfi closed this Feb 17, 2016

lsmith77 removed the wip/poc label Feb 17, 2016

[WIP] Symfony3 #331

[WIP] Symfony3 #331

Uh oh!

Conversation

hacfi commented Jan 30, 2016

Uh oh!

hacfi commented Jan 30, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hacfi commented Jan 30, 2016

Uh oh!

hacfi commented Jan 30, 2016

Uh oh!

wouterj commented Jan 30, 2016

Uh oh!

wouterj commented Jan 30, 2016

Uh oh!

hacfi commented Jan 31, 2016

Uh oh!

hacfi commented Jan 31, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dbu commented Feb 1, 2016

Uh oh!

hacfi commented Feb 1, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dbu commented Feb 1, 2016

Uh oh!

hacfi commented Feb 1, 2016

Uh oh!

hacfi commented Feb 16, 2016

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!

wouterj commented Feb 16, 2016

Uh oh!

hacfi commented Feb 16, 2016

Uh oh!

dbu commented Feb 17, 2016

Uh oh!

hacfi commented Feb 17, 2016

Uh oh!

Uh oh!