Add symfony 3.0 support #752

GuilhemN · 2015-11-11T09:38:10Z

This PR adds symfony 3.0 support.

GuilhemN · 2015-11-11T09:39:00Z

composer.json

+            "type": "vcs",
+            "url": "https://github.com/Ener-Getick/DunglasApiBundle.git"
+        }
+    ],


Must be removed once api-platform/core#323 is merged.

GuilhemN · 2015-11-14T15:47:51Z

ping @willdurand

weaverryan · 2015-11-23T18:06:38Z

@Ener-Getick This is GREAT work, but your tests are failing:

2.8 is failing due to Composer
3.0 is failing due to some deprecated things not being fixed yet - for example the @ symbols in https://github.com/nelmio/NelmioApiDocBundle/blob/master/Tests/Fixtures/app/config/routing.yml aren't all quoted yet.

Could you take a look at the tests please?

Thanks!

GuilhemN · 2015-11-23T20:12:08Z

I will try to fix the remaining deprecations tomorrow, I didn't localize all of them yet.

GuilhemN · 2015-11-28T17:05:39Z

@xabbuh @fabpot do you know why there is no stable release of symfony/form on packagist ?

xabbuh · 2015-11-28T17:23:19Z

Not sure what you mean. I see lots of stable versions.

GuilhemN · 2015-11-28T17:24:50Z

Apparently this is fixed. A few minutes ago there were only the dev versions available.

GuilhemN · 2015-11-28T18:40:24Z

I fixed several errors but the behavior of the symfony forms seems to be changed in sf 3.0 and so the NelmioApiDocBundle behavior ... :/

weaverryan · 2015-11-30T14:59:51Z

@Ener-Getick why did you close this? We can find a solution for supporting the form behavior across multiple versions - SonataAdminBundle also had to support this problem. I would love for you to continue your nice work to completion!

GuilhemN · 2015-11-30T15:04:54Z

I didn't have any response so I thought you abandon. I'm not really involved in symfony core and in symfony/form so I don't know how we can fix this and why this behavior changed :-/

xabbuh · 2015-11-30T15:09:33Z

I'll try to help you with this after SymfonyCon (I.e. next week). Please ping me then if I forget about it.

weaverryan · 2015-11-30T15:10:02Z

👍 @xabbuh

And @Ener-Getick what are the problems that you're seeing? I noticed that you have the LegacyFormHelper, which is a nice way to handle the text -> TextType style of changes.

GuilhemN · 2015-11-30T15:20:17Z

Thanks @xabbuh :-)

@weaverryan the output of the formatters has changed (an example) I don't know why... I'll try to search again this week.

But I'm wondering if it wouldn't be easier to split this PR because it introduces a lot of changes to test at the same time...

weaverryan · 2015-11-30T15:27:25Z

Hmm, yea, I don't know about the formatting change - that is strange.

If it's easier for you to split across multiple PR's and get things working, you could do that. But I don't think it's too hard for a reviewer to review.

GuilhemN · 2015-11-30T18:06:55Z

The problem is that I've made too many changes and I don't know when failures appeared... I'll create a new PR for everything already stable.

Finally, I'll just revert the last changes of this PR to found when errors appeared. No need of a new PR ^^

GuilhemN · 2015-11-30T20:00:55Z

There are already several errors with symfony 2.8 (see the build). I'll try to understand why this tests fail.

GuilhemN · 2015-11-30T20:44:58Z

Tests/Fixtures/Form/ImprovedTestType.php

+            ->add('c1', $choiceType, array('choices' => array('m' => 'Male', 'f' => 'Female'), 'choices_as_values' => true))
+            ->add('c2', $choiceType, array('choices' => array('m' => 'Male', 'f' => 'Female'), 'choices_as_values' => true, 'multiple' => true))
+            ->add('c3', $choiceType, array('choices' => array()))
+            ->add('c4', $choiceType, array('choices' => array('foo' => 'bar', 'bazgroup' => array('baz' => 'Buzz')), 'choices_as_values' => true))


The users will have to update their form with choices_as_values if they want to use symfony 2.8 :/

GuilhemN · 2015-12-01T12:34:13Z

The last commit speeds up travis of ~50% !
before / after

GuilhemN · 2015-12-01T13:53:49Z

The remaining errors seem unrelated to this PR. Needs review

luispabon · 2015-12-01T15:05:53Z

Parser/FormTypeParser.php

+            } else {
+                throw new \InvalidArgumentException('Unsupported form type class.');
+            }
+        }

        $name = array_key_exists('name', $item) ? $item['name'] : $form->getName();


Form::getName has been removed from Symfony 3, use getBlockPrefix instead

Thanks, this line is apparently not covered by the tests...

weaverryan · 2015-12-01T16:32:39Z

The asset() function failure on 3.0 should be fixed by adding asset: ~ into default.yml under the framework key: https://github.com/nelmio/NelmioApiDocBundle/blob/master/Tests/Fixtures/app/config/default.yml

But, that will also break the tests in 2.7 and lower. So in practice, we may need to add it in AppKernel, detect the Symfony version, and only add the asset key (this can be done via a closure) for 2.8 and higher.

Nice job btw!

GuilhemN · 2015-12-03T14:44:38Z

You're welcome ;-)

And yeah, see this. Can you relaunch Travis? Otherwise I'll do it tonight.

willdurand · 2015-12-03T15:54:12Z

@Ener-Getick that's what I did (triggering travis-ci again), but it is still failing...

GuilhemN · 2015-12-03T15:59:39Z

Yeah just see that dunglas/api-bundle is under 2.0.x-dev

willdurand · 2015-12-03T16:07:47Z

oh yes... @dunglas can we safely rely on 2.0@dev?

dunglas · 2015-12-03T16:39:35Z

No we can't but AFAIK the @Ener-Getick patch (thx for the good work) has been merged in 1.0@dev.

GuilhemN · 2015-12-03T17:51:12Z

I just discovered that packagist detects the branches named \d.\d the problem is that the last dev release is v1.1.0-beta.1 and 1.0@dev < v1.1.0-beta.1.

@dunglas could you rename your branch to 1.1 ?

dunglas · 2015-12-04T00:06:03Z

I've just created a 1.1 branch (it's cleaner I think). It should work when #764 is merged.

dunglas · 2015-12-04T07:00:18Z

In fact renaming the 1.0 branch to 1.x do the trick. Can you run Travis again?

GuilhemN · 2015-12-04T07:26:08Z

It works thanks @dunglas

Add symfony 3.0 support

willdurand · 2015-12-04T08:09:35Z

awesome, thanks!

dunglas · 2015-12-04T08:19:51Z

Thanks a lot @Ener-Getick

rande · 2015-12-04T08:30:38Z

GG!

luispabon · 2015-12-04T09:03:54Z

Thank you 👍

1 Neal Street London WC2H 9QL

www.rightster.com | @Rightster https://twitter.com/rightster | LinkedIn
https://www.linkedin.com/company/rightster

Rightster Limited (incorporated in England with company number 07634543)
has its registered office at 1 Neal Street London WC2H 9QL. Rightster
Limited is a subsidiary of Rightster Group plc (incorporated in England and
Wales with company number 08754680).

GuilhemN · 2015-12-04T16:45:42Z

Awesome thank you all for your help

teohhanhui · 2015-12-08T07:31:57Z

I get this deprecation notice on Symfony 2.8:

The alias option of the form.type_extension tag is deprecated since version 2.8 and will be removed in 3.0. Use the extended_type option instead.

This is because of the "alias" attribute here:

NelmioApiDocBundle/Resources/config/services.xml

Line 40 in 1df3ddf

    
           <tag name="form.type_extension" alias="form" extended-type="Symfony\Component\Form\Extension\Core\Type\FormType" />

GuilhemN · 2015-12-08T07:35:23Z

Hum this deprecation has probably been added later. I'll fix it this afternoon.

GuilhemN · 2015-12-08T12:07:06Z

Which version of the bundle do you use ? This should not trigger a deprecation see here

teohhanhui · 2015-12-08T12:11:30Z

@Ener-Getick You're right. It must be caused by another bundle then... (The way deprecation warnings are logged is pretty unhelpful.)

weaverryan · 2015-12-08T12:17:01Z

@teohhanhui you can open an issue in symfony/symfony if a deprecation warning isn't clear enough - it's pretty common that many can be improved to contain more information (in this case, if the service id were in the message, that would help a lot).

willdurand · 2015-12-08T13:22:22Z

@teohhanhui v2.11.1 is released and contains @Ener-Getick's fix.

GuilhemN · 2015-12-08T13:24:37Z

I forgot to specify, it's a fix for a deprecation triggered in the tests not in production :x

willdurand · 2015-12-08T13:26:28Z

@Ener-Getick yep, but it documents how to fix this deprecation :)

teohhanhui · 2015-12-08T14:13:03Z

Uhh, yes. I rechecked and it turns out I was using 2.10.3. The deprecation notice I was getting was already fixed by this PR (in 2.11.0). Silly me!

OskarStark · 2016-02-10T08:35:51Z

👍

GuilhemN reviewed Nov 11, 2015
View reviewed changes

GuilhemN closed this Nov 30, 2015

GuilhemN deleted the SF3 branch November 30, 2015 15:02

GuilhemN restored the SF3 branch November 30, 2015 15:02

GuilhemN reopened this Nov 30, 2015

GuilhemN reviewed Nov 30, 2015
View reviewed changes

willdurand added WIP Work in progress issues that are not ready for implementation accepted labels Dec 1, 2015

willdurand added this to the 2.12.0 milestone Dec 1, 2015

GuilhemN changed the title ~~[WIP] Add symfony 3.0 support~~ Add symfony 3.0 support Dec 1, 2015

luispabon reviewed Dec 1, 2015
View reviewed changes

mbabker mentioned this pull request Dec 3, 2015

Symfony 3 compatibility sonata-project/SonataMediaBundle#913

Merged

dunglas mentioned this pull request Dec 4, 2015

[DO NOT MERGE] Require dunglas/api-bundle:1.1@dev #764

Closed

Optimize travis

0461f5c

willdurand added a commit that referenced this pull request Dec 4, 2015

Merge pull request #752 from Ener-Getick/SF3

1df3ddf

Add symfony 3.0 support

willdurand merged commit 1df3ddf into nelmio:master Dec 4, 2015

GuilhemN deleted the SF3 branch December 4, 2015 16:45

Add symfony 3.0 support #752

Add symfony 3.0 support #752

Conversation

GuilhemN commented Nov 11, 2015

GuilhemN Nov 11, 2015

Choose a reason for hiding this comment

GuilhemN commented Nov 14, 2015

weaverryan commented Nov 23, 2015

GuilhemN commented Nov 23, 2015

GuilhemN commented Nov 28, 2015

xabbuh commented Nov 28, 2015

GuilhemN commented Nov 28, 2015

GuilhemN commented Nov 28, 2015

weaverryan commented Nov 30, 2015

GuilhemN commented Nov 30, 2015

xabbuh commented Nov 30, 2015

weaverryan commented Nov 30, 2015

GuilhemN commented Nov 30, 2015

weaverryan commented Nov 30, 2015

GuilhemN commented Nov 30, 2015

GuilhemN commented Nov 30, 2015

GuilhemN Nov 30, 2015

Choose a reason for hiding this comment

GuilhemN commented Dec 1, 2015

GuilhemN commented Dec 1, 2015

luispabon Dec 1, 2015

Choose a reason for hiding this comment

GuilhemN Dec 1, 2015

Choose a reason for hiding this comment

weaverryan commented Dec 1, 2015

GuilhemN commented Dec 3, 2015

willdurand commented Dec 3, 2015

GuilhemN commented Dec 3, 2015

willdurand commented Dec 3, 2015

dunglas commented Dec 3, 2015

GuilhemN commented Dec 3, 2015

dunglas commented Dec 4, 2015

dunglas commented Dec 4, 2015

GuilhemN commented Dec 4, 2015

willdurand commented Dec 4, 2015

dunglas commented Dec 4, 2015

rande commented Dec 4, 2015

luispabon commented Dec 4, 2015

GuilhemN commented Dec 4, 2015

teohhanhui commented Dec 8, 2015

GuilhemN commented Dec 8, 2015

GuilhemN commented Dec 8, 2015

teohhanhui commented Dec 8, 2015

weaverryan commented Dec 8, 2015

willdurand commented Dec 8, 2015

GuilhemN commented Dec 8, 2015

willdurand commented Dec 8, 2015

teohhanhui commented Dec 8, 2015

OskarStark commented Feb 10, 2016