-
Notifications
You must be signed in to change notification settings - Fork 2.3k
863 @WakeLock #1130
863 @WakeLock #1130
Conversation
|
TODO:
|
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.
Remove public static final.
|
Hey. First thing first. The feature can change during the review, so you should create the wiki page after the PR is merged, or a final OK has been noted, so you do not have to rewrite it. But THX for being super-precise here. |
|
@WonderCsabo tests will follow tomorrow depending on my free time. i did the wiki first as it was faster than writing tests. ;) if i have to rewrite the wiki then its my own fault (actually i did not planned to do the wiki right now, instead i planned to do it tomorrow together with the tests but then ... __poof** i was editing the wiki 😄 ) |
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 am not sure we should copy the JavaDoc here, @see can be enough. We should not describe android features, we should just tell how our wrapper uses them.
|
updated the PR |
|
tests added and wiki updated for the refactored |
|
actually it is only one very simple test. this is for two reasons
|
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.
Maybe you can add a callback to this Activity which gets called in this method. You add a callback in the test, and in that you can assert the WakeLock is held.
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.
good idea
|
I added dodgex/androidannotations#1 which fully tests this feature with a little trick. Please merge to this branch. |
|
merged. |
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.
Do not import for JavaDoc, use fully qualified class 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.
updated
|
You removed my commit with your last rebase, but that is no problem. I am thinking now It is cleaner to add that in a different PR. @yDelouis I reviewed this PR and it is ready to be merged, but we have to wait for 3.1. |
|
@WonderCsabo whoops. sorry, not sure how this happened. if you want it as your commit, feel free to open a new PR on my fork otherwise can readd the changes to my PR. maybe it is even enough if you revert the branch deletion. |
|
I mean it is more clear if i add a PR to this repo directly with my commit. |
|
Ok. |
|
@dodgex please rebase. |
- to allow checking for any permission - added a warning for library projects instead of ignoring the permission check
|
rebased on curret develop. |
|
Finally merged this, and also added the tests. |
|
I merged your changes from the wiki. I rebased and squashed your commits, so you should reset your wiki to our HEAD. |
|
Something changed about the wiki editor, because i just found out that i also changed LF to CRLF here. Everybody should use Unix... (including me). So in the next time, we should commit from the git command line to be sure about the line endings. |
adds @WakeLock annotation see #863 for details