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

nette/DI 3.0.2 compatiblity #27

Merged
merged 3 commits into from Feb 6, 2020
Merged

nette/DI 3.0.2 compatiblity #27

merged 3 commits into from Feb 6, 2020

Conversation

chapcz
Copy link
Contributor

@chapcz chapcz commented Dec 27, 2019

@MartinLojda
Copy link

Ping @enumag .

@enumag
Copy link
Member

enumag commented Feb 4, 2020

I don't really understand. Is there a BC break in 3.0.2 specifically (as in everything is fine with 3.0.1)? If so then it should be reported to nette/di and reverted.

Or is this a problem with 3.0.0+?

@MartinLojda
Copy link

I believe this is the change in nette/di that breaks it:

nette/di@v3.0.1...v3.0.2#diff-6682ea383870132c0a5bfc2133f557d1R456

Not really sure it's a BC as it's pretty low level function...

@enumag
Copy link
Member

enumag commented Feb 5, 2020

The Resolver class is marked as @internal so it's not actually a BC break. Which means the correct fix is to refactor this package to not use it.

@chapcz chapcz closed this Feb 5, 2020
@chapcz chapcz reopened this Feb 5, 2020
@chapcz
Copy link
Contributor Author

chapcz commented Feb 5, 2020

Is ok to pick these two static methods from DI Resolver class and put them into this package?

private static function autowireArgument(\ReflectionParameter $parameter, callable $getter)
public static function autowireArguments(\ReflectionFunctionAbstract $method, array $arguments, callable $getter): array

@enumag
Copy link
Member

enumag commented Feb 5, 2020

Hmm that's not really something we should duplicate... ok maybe I should merge it as is then. Can you please verify that your fix doesn't break Nette 3.0.0 and 3.0.1?

@chapcz
Copy link
Contributor Author

chapcz commented Feb 5, 2020

Good point - not working for those versions. Is ok to set a minimal version in the composer?
"nette/di": "^3.0.2"

Co-Authored-By: Jáchym Toušek <enumag@gmail.com>
@enumag
Copy link
Member

enumag commented Feb 5, 2020

I guess so. Do it.

@chapcz
Copy link
Contributor Author

chapcz commented Feb 5, 2020

done

@enumag enumag merged commit f82cde0 into Kdyby:master Feb 6, 2020
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.

None yet

3 participants