Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 28457002: Only execute first run logic once (Closed)

Created:
Nov. 7, 2013, 2:05 p.m. by Felix Dahlke
Modified:
Nov. 8, 2013, 2:36 p.m.
Visibility:
Public.

Description

This fixes a bug in the libadblockplus branch: The first run code (most notably that showing the message) is executed whenever the activity is brought to front, while it should really only be executed once.

Patch Set 1 : #

Total comments: 2

Patch Set 2 : Move firstRunActionsPending = false up #

Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -9 lines) Patch
M src/org/adblockplus/android/Preferences.java View 1 2 chunks +10 lines, -9 lines 0 comments Download

Messages

Total messages: 3
Felix Dahlke
Nov. 7, 2013, 2:25 p.m. (2013-11-07 14:25:44 UTC) #1
Wladimir Palant
LGTM, feel free to decide whether you want to address the nit. http://codereview.adblockplus.org/28457002/diff/30002/src/org/adblockplus/android/Preferences.java File src/org/adblockplus/android/Preferences.java ...
Nov. 8, 2013, 6:08 a.m. (2013-11-08 06:08:20 UTC) #2
Felix Dahlke
Nov. 8, 2013, 8:37 a.m. (2013-11-08 08:37:36 UTC) #3
http://codereview.adblockplus.org/28457002/diff/30002/src/org/adblockplus/and...
File src/org/adblockplus/android/Preferences.java (right):

http://codereview.adblockplus.org/28457002/diff/30002/src/org/adblockplus/and...
src/org/adblockplus/android/Preferences.java:193: firstRunActionsPending =
false;
On 2013/11/08 06:08:20, Wladimir Palant wrote:
> IMHO it's better to group related operations - I would set that immediately
> after setting firstRun variable.

Hehe, I was actually pondering that :) I'd prefer it up there, but concluded
that it might be misleading to have firstRunActionsPending set to false before
any first run actions have been executed. But I guess it's still better.

Powered by Google App Engine
This is Rietveld