Skip to content
This repository was archived by the owner on Feb 26, 2023. It is now read-only.

Conversation

@dodgex
Copy link
Member

@dodgex dodgex commented Sep 3, 2014

adds @WakeLock annotation see #863 for details

@dodgex
Copy link
Member Author

dodgex commented Sep 3, 2014

TODO:

  • tests
  • wiki updated

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove public static final.

@dodgex
Copy link
Member Author

dodgex commented Sep 3, 2014

@WonderCsabo
Copy link
Member

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.
Also writing the test is always more important than the wiki.

@dodgex
Copy link
Member Author

dodgex commented Sep 3, 2014

@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 😄 )

Copy link
Member

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.

@dodgex
Copy link
Member Author

dodgex commented Sep 4, 2014

updated the PR

@dodgex
Copy link
Member Author

dodgex commented Sep 4, 2014

tests added and wiki updated for the refactored Level and Flags enums. i also added a note about the known issue related to @Trace and return values.

@dodgex
Copy link
Member Author

dodgex commented Sep 4, 2014

actually it is only one very simple test. this is for two reasons

  1. robolectric does not offer much stuff to test WakeLocks
  2. i have no idea what else could be tested. ;)

Copy link
Member

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea

@WonderCsabo
Copy link
Member

I added dodgex/androidannotations#1 which fully tests this feature with a little trick. Please merge to this branch.

@dodgex
Copy link
Member Author

dodgex commented Sep 5, 2014

merged.

Copy link
Member

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

@WonderCsabo
Copy link
Member

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.

@dodgex
Copy link
Member Author

dodgex commented Sep 5, 2014

@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.

@WonderCsabo
Copy link
Member

I mean it is more clear if i add a PR to this repo directly with my commit.

@dodgex
Copy link
Member Author

dodgex commented Sep 6, 2014

Ok.

@WonderCsabo
Copy link
Member

@dodgex please rebase.

- to allow checking for any permission
- added  a warning for library projects instead of ignoring the permission check
@dodgex
Copy link
Member Author

dodgex commented Sep 20, 2014

rebased on curret develop.

WonderCsabo added a commit that referenced this pull request Sep 20, 2014
@WonderCsabo WonderCsabo merged commit 56d85fa into androidannotations:develop Sep 20, 2014
@dodgex dodgex deleted the 863_WakeLock branch September 20, 2014 17:10
@WonderCsabo
Copy link
Member

Finally merged this, and also added the tests.

@WonderCsabo
Copy link
Member

I merged your changes from the wiki. I rebased and squashed your commits, so you should reset your wiki to our HEAD.

@WonderCsabo
Copy link
Member

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.

@yDelouis yDelouis added this to the 3.2 milestone Sep 21, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants