-
Notifications
You must be signed in to change notification settings - Fork 2.3k
1232 mutliple actions for receiveraction #1234
1232 mutliple actions for receiveraction #1234
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think it is a good idea to catch intents with action as the method name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, you are right, but that was the old behaviour. changing this might break existing apps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooops, i see. We missed that in the initial review of this feature...
It should stay, then, but we will change the behavior in 4.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall i open an issue and prepare a PR (based on this one)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically a PR should not depend on other PRs, but we can make an exception here. Thanks!
|
I just realized Otherwise, this PR looks good to me. |
|
currently it seems required for stuff aditional injection. e.g. i have a and when i see the generated code, i'm afraid that the Broadcast receiver is potentially a (VERY) big source of unexpected behaviour/bugs. injection happens for each recived intent... let asume we have a
|
|
You are right... |
|
OK, i just realized. |
|
yeah. but i'm not sure if that is a good way. |
|
BTW that is another issue. Can you open a new ticket to discuss that? Back to my original question: generating |
|
i have no idea how to do that the best way. currently i'm testing if it is possible to just pass the context to |
|
added commit + rebased on develop |
|
Thanks. The |
|
Can you add a commit which lazily generates the |
|
@WonderCsabo i think the for String action = ...
if(action.equals(action)) {
}
if(action2.equals(action)) {
}
String dataSchemes = ..
if(action3.equals(action) && dataschemes.equal(scheme)) {
}is that ok? |
|
Action is not required if one not uses the |
|
@dodgex can you address my comments? |
|
oh. sure. i have overseen the last comment with about |
|
I think there is no other. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move the strings into constants.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
…eraction 1232 mutliple actions for receiveraction
|
Thanks for this contribution. Can you update the wiki? |
|
Wiki merged, thanks! |
implements #1232