allow method injection #204 #1457
Conversation
|
related to #204 |
|
Nice start! Can you please rebase this onto |
|
rebased |
There was a problem hiding this comment.
There is a typo in the method name.
|
Seems good. However, if you want to extend this to all injection annotation, I think we will need to create a helper in order not to have to duplicate this code structure in all Handlers. |
|
i started this as PoC to see what is possible. but yeah, how that we found that it is possible the plan is to extend it to (if possible) all injections |
|
@WonderCsabo @yDelouis i updated the PR. i refactored the code into a helper class and added method injection for feedback is highly appreciated! |
There was a problem hiding this comment.
These fields should be private.
There was a problem hiding this comment.
should they? in other tests similar fields are default too. thats why i left the visibiliy.
There was a problem hiding this comment.
Never mind. The fields are packapege private because we have to access them from the test class.
I just said it because we want method injections to be able to inject to private fields (at least this is on of the reasons). But we still have to access the fields in the test class, so just forget my comment.
There was a problem hiding this comment.
i just realized that there is no consistency at all in the test classes. the BeanInjctedActivity has all fields public
There was a problem hiding this comment.
These fields should be pp, but it is not a big deal.
|
I am wondering, could it be easier to create a common super |
|
This should be rebased. |
|
rebased an adressed some of the comments |
|
about the common super class. first the question is what injections should be supported? e.g. not sure what the purpose of |
|
here is a list of annotations that ca be applied to fields. the checked are already supported.
🚫 The last annotations are not related to injection. :) |
|
I think all listed annotations are valid. And do not forget annotations from plugins ( |
|
so i need to find/create a common level of |
|
The helper class is also fine for me, if finding a common super class is
not possible.
|
|
currently broken, but that should be fixable with #1566 |
|
I'm okay with the listed annotations. If we missed some, we could add them later. |
|
@dodgex this is no longer blocked. |
|
@WonderCsabo i know, i already rebased after the blocker PR was merged. but i had no time yet to continue work on this. |
|
|
|
same for |
|
Some annotations are missing from plugins (OrmLiteDao, RestService,? ) |
|
arg... yeah. i think i have not checked the plugins when generating the above list of annotations todo. |
|
Are you planning to add those to this PR as well? |
|
yeah. if it is ok. |
…mutltiple injections
|
@dodgex can you update the wiki sometime? |
|
I'll try to do it the next few days. |
|
Checklist for Wiki Updates:
|
|
wiki on my fork is updated. also i have added the commit |
|
added |
|
@WonderCsabo I assume you missed that i updated the wiki? |
|
Thanks for pinging. I will check it in a couple of hours.
|
|
no problem |
|
I merged your wiki changes. Thanks again for your great work! |
this is the WIP branch for method injection. in its initial implementation only for
@Beanbut planned to be expanded to most (if not all) injection related annotations.