Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Removes the callback from the handler when the activity is destroyed #361

Open
wants to merge 3 commits into
base: bug-fixes
Choose a base branch
from
Open

Removes the callback from the handler when the activity is destroyed #361

wants to merge 3 commits into from

Conversation

saketkumar
Copy link

Now it wouldn't cause a leak when we don't need the handler anymore.

@opticod Review and tell me if you need any changes. :)

opticod
opticod previously approved these changes Jan 17, 2017
View reviewed changes
Copy link
Collaborator

@opticod opticod left a comment

Choose a reason for hiding this comment

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

Thanks @saketkumar95

@opticod
Copy link
Collaborator

opticod commented Jan 17, 2017

@saketkumar95 It looks that you have created the PR targeted to master branch. Can you please redirect to bug-fixes branch.

opticod
opticod reviewed Jan 17, 2017
View reviewed changes
source-code/app/src/main/java/org/buildmlearn/toolkit/activity/HomeActivity.java
@Override
protected void onDestroy() {
super.onDestroy();
if (handler != null)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also reformat your new changes.

@opticod opticod dismissed their stale review January 17, 2017 17:17

need to change target branch

@saketkumar saketkumar changed the base branch from master to bug-fixes January 17, 2017 21:36
@saketkumar
Copy link
Author

@opticod Done. :)

@opticod
Copy link
Collaborator

opticod commented Jan 19, 2017 •
edited
Loading

@saketkumar95 , looks like you haven't reformatted your code.
We follow

 if () {

}

formatting.Please reformat it. You can automatically do it in Android studio.

@saketkumar
Copy link
Author

@opticod I have reformatted the code. And i saw some more which are not formatted so i thought to reformat the whole project. What do you think about this?

opticod
opticod previously approved these changes Jan 19, 2017
View reviewed changes
Copy link
Collaborator

@opticod opticod left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Lets reformat the code before merging bug-fixes to master, but not now.

@saketkumar saketkumar mentioned this pull request Jan 21, 2017
Closed
@opticod opticod force-pushed the bug-fixes branch 3 times, most recently from a3993ce to 6eea6bd Compare January 29, 2017 14:13
@opticod
Copy link
Collaborator

opticod commented Feb 7, 2017

@saketkumar95 , Are you on this #353

@saketkumar
Copy link
Author

I just missed this. Will update the branch tonight. :)

@opticod
Copy link
Collaborator

opticod commented Sep 1, 2017

Are you still on this ? Will soon start reviewing and merging the PRs. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@opticod opticod opticod left review comments

Assignees
No one assigned
Labels
request changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@saketkumar @opticod @saketkumar95