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

Constructor overload for object creation during dependency injection #3356

Merged
merged 10 commits into from Apr 10, 2019

Conversation

Projects
None yet
3 participants
@nickrandolph
Copy link
Contributor

commented Apr 5, 2019

What kind of change does this PR introduce? (Bug fix, feature, docs update...)

Perf improvement

⤵️ What is the current behavior?

Types need to be constructed using reflection or via an Func delegate

🆕 What is the new behavior (if this is a feature change)?

Can supply a Func that takes type resolved arguments and a call to a constructor, eliminating the need for reflection to inspect the type being created

💥 Does this PR introduce a breaking change?

No

🐛 Recommendations for testing

Run all unit tests

📝 Links to relevant issues/docs

🤔 Checklist before submitting

@nickrandolph nickrandolph marked this pull request as ready for review Apr 6, 2019

@nickrandolph nickrandolph requested a review from MvvmCross/core as a code owner Apr 6, 2019

nickrandolph added some commits Apr 6, 2019

@nickrandolph

This comment has been minimized.

Copy link
Contributor Author

commented Apr 7, 2019

@martijn00 this has been updated to use extension methods instead of changing the interface

@Cheesebaron

This comment has been minimized.

Copy link
Member

commented Apr 9, 2019

Need to figure out why that Navigation Test fails on CI, but it is not your fault. This PR looks good to me!

@martijn00 martijn00 added this to the 6.2.4 milestone Apr 10, 2019

@martijn00 martijn00 merged commit 35821a3 into MvvmCross:develop Apr 10, 2019

2 of 4 checks passed

CodeFactor 1 issue found.
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
WIP Legacy commit status override — see details
Details
license/cla All CLA requirements met.
Details
@nickrandolph

This comment has been minimized.

Copy link
Contributor Author

commented Apr 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.