Skip to content

Conversation

@althaus
Copy link
Contributor

@althaus althaus commented Jun 3, 2015

This checks if "setFactory" is available in favor of "setFactoryClass".

This checks if "setFactory" is available in favor of "setFactoryClass".
@MasterB
Copy link

MasterB commented Jun 3, 2015

+1

1 similar comment
@pzorn
Copy link

pzorn commented Jun 4, 2015

+1

@spolischook
Copy link
Contributor

Update dependencies in composer.json

@spolischook spolischook self-assigned this Jun 5, 2015
@spolischook spolischook added this to the 0.2.0 milestone Jun 5, 2015
@althaus
Copy link
Contributor Author

althaus commented Jun 5, 2015

@spolischook You mean enforcing Symfony 2.7 in the deps? My fix doesn't change the minimal required version.

@jbgomond
Copy link

jbgomond commented Jun 6, 2015

👍

@spolischook
Copy link
Contributor

it's add Symfony 2.7 support? Other parts is work good with 2.7?

@jbgomond
Copy link

jbgomond commented Jun 7, 2015

Yes, this was the code triggering deprecated notices.

Copy link
Contributor

Choose a reason for hiding this comment

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

for this code use separate method with

/** 
  * @deprecated from symfony 2.7
  **/ 

comment above it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think the code should trigger a deprecation notice as there is nothing which is deprecated itself. It will run fine with Symfony <2.7 and >=2.7.

The FriendsOfSymfony solved it the same way:

FriendsOfSymfony/FOSUserBundle@0f085ab

@althaus
Copy link
Contributor Author

althaus commented Jun 8, 2015

Other parts is work good with 2.7?

Yes, I haven't found any other issue yet.

Symfony 2.7 deprecates the setFactoryClass and setFactoryMethod methods in favor of a combined setFactory call:

https://github.com/symfony/symfony/blob/2.7/UPGRADE-3.0.md#dependencyinjection

@jbgomond
Copy link

Could somebody merge it ? ping @cystbear

@ghost
Copy link

ghost commented Jun 12, 2015

+1. Could we merge this upstream? ping @spolischook

@cystbear
Copy link
Contributor

@althaus does this stuff works?
/cc @MJBGO @edvinasme

@althaus
Copy link
Contributor Author

althaus commented Jun 15, 2015

@cystbear Yes, our fork runs fine with 2.7 in our dev environment. The fix isn't big magic.

@jbgomond
Copy link

👍 Works for me, it is just replacing the old methods by the new ones ;)

spolischook added a commit that referenced this pull request Jun 15, 2015
Upgrade DependencyInjection to Symfony 2.7
@spolischook spolischook merged commit 7d34e1d into Exercise:master Jun 15, 2015
@spolischook
Copy link
Contributor

Thanks for contributing!

@althaus
Copy link
Contributor Author

althaus commented Jun 19, 2015

@spolischook Thanks for the merge. Could you tag a new release for Composer?

@althaus althaus deleted the fix-definition branch June 19, 2015 10:32
@HeahDude HeahDude removed this from the 2.0 milestone Dec 11, 2019
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.

7 participants