Skip to content

Conversation

@charabaruk
Copy link
Contributor

For RESOLVE_OPEN_GENERICS add a new "__requestedType" parameter when calling a factory method providing the type object for the type being resolved. This allows factory methods for registered open generic types to properly resolve the particular closed generic type requested.

@niemyjski
Copy link
Collaborator

Could you please add a test for this

@charabaruk
Copy link
Contributor Author

I'll see what I can do but it probably won't be for a few days given my work schedule.

@charabaruk charabaruk force-pushed the open-generics-provide-requested-type branch from 86ddc20 to ee3c4cb Compare November 23, 2020 17:42
@charabaruk
Copy link
Contributor Author

@niemyjski I've added a test for __requestedType. In commit ee3c4cb I've also turned on RESOLVE_OPEN_GENERICS tests in the test project, however Resolve_RegisteredOpenGenericInParent_CanBeResolvedByChild() which was failing before this work continues to fail so should be ignored in test results.

@niemyjski
Copy link
Collaborator

Thanks for the updates. Is there any way we can resolve the failing tests, they were all green before this but I get enabling a flag may break others but we need to keep the build passing.

@charabaruk
Copy link
Contributor Author

charabaruk commented Nov 23, 2020

I see two options:

  1. Merging Fix child resolve for types with generic interfaces #137 and resolving the merge conflict between it and this PR in this PR's favour
  2. Rolling back ee3c4cb, so that the RESOLVE_OPEN_GENERICS tests aren't run

I have no problem with bumping this PR's branch back to b8f7bd2 and providing a separate PR for enabling the RESOLVE_OPEN_GENERICS tests. That other PR would by necessity include #137's commits within it though, due to the conflict that would otherwise occur if both that and this are merged together.

Let me know which way we should take this.

@niemyjski
Copy link
Collaborator

I'd like to go with #1, is there any chance you could fork his fork and submit a new pr with the requested changes?

@charabaruk
Copy link
Contributor Author

I'll give it a shot during my lunch break today.

@charabaruk
Copy link
Contributor Author

@niemyjski I just created #141 which is the same as #137 with commit ee3c4cb cherry-picked onto it. That takes care of moving RESOLVE_OPEN_GENERICS to the project file, but I can't answer your other concern with #137. After it's merged in, I can rebase this PR's branch and it should pass all tests (including the new one for its functionality).

@niemyjski
Copy link
Collaborator

Thanks for the update! I just merged it in, let me know when you rebase and this is good to be merged in.

charabaruk and others added 2 commits November 26, 2020 09:12
For `RESOLVE_OPEN_GENERICS` add a new "__requestedType" parameter when
calling a factory method providing the type object for the type being
resolved. This allows factory methods for registered open generic types
to properly resolve the particular closed generic type requested.
@charabaruk charabaruk force-pushed the open-generics-provide-requested-type branch from ee3c4cb to ae7e0dd Compare November 26, 2020 14:14
@charabaruk
Copy link
Contributor Author

@niemyjski and we are good to go!

@niemyjski niemyjski merged commit 9914042 into grumpydev:master Nov 29, 2020
@niemyjski
Copy link
Collaborator

Thanks for the pr

@charabaruk charabaruk deleted the open-generics-provide-requested-type branch November 29, 2020 17:06
@khanyuriy
Copy link

why this is only for conditional compilation?
if we delegate service location dont we need to tell what specific type was requested ?

@niemyjski
Copy link
Collaborator

@khanyuriy Can you please elaborate further

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.

3 participants