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

Issue 9423010: ABP/Android Better icon hide (Closed)

Created:
Feb. 20, 2013, 12:08 p.m. by Andrey Novikov
Modified:
Jan. 4, 2016, 12:35 p.m.
Reviewers:
Thomas Greiner
Visibility:
Public.

Description

ABP/Android Better icon hide

Patch Set 1 #

Total comments: 7
Unified diffs Side-by-side diffs Delta from patch set Stats (+125 lines, -58 lines) Patch
M res/values/strings.xml View 1 chunk +2 lines, -1 line 0 comments Download
M src/org/adblockplus/android/AdblockPlus.java View 12 chunks +37 lines, -9 lines 1 comment Download
M src/org/adblockplus/android/Preferences.java View 2 chunks +21 lines, -2 lines 1 comment Download
M src/org/adblockplus/android/ProxyService.java View 9 chunks +65 lines, -46 lines 4 comments Download

Messages

Total messages: 2
Andrey Novikov
Feb. 20, 2013, 12:08 p.m. (2013-02-20 12:08:41 UTC) #1
Thomas Greiner
Feb. 27, 2013, 4:34 p.m. (2013-02-27 16:34:29 UTC) #2
only minor changes :)

http://codereview.adblockplus.org/9423010/diff/1/src/org/adblockplus/android/...
File src/org/adblockplus/android/AdblockPlus.java (right):

http://codereview.adblockplus.org/9423010/diff/1/src/org/adblockplus/android/...
src/org/adblockplus/android/AdblockPlus.java:256: *
How did that happen? I noticed that there are 8 occurences of such trailing
whitespace that were added in this commit.

http://codereview.adblockplus.org/9423010/diff/1/src/org/adblockplus/android/...
File src/org/adblockplus/android/Preferences.java (right):

http://codereview.adblockplus.org/9423010/diff/1/src/org/adblockplus/android/...
src/org/adblockplus/android/Preferences.java:359: {
mind the indentation level

http://codereview.adblockplus.org/9423010/diff/1/src/org/adblockplus/android/...
src/org/adblockplus/android/Preferences.java:370: {
mind the indentation level

http://codereview.adblockplus.org/9423010/diff/1/src/org/adblockplus/android/...
File src/org/adblockplus/android/ProxyService.java (right):

http://codereview.adblockplus.org/9423010/diff/1/src/org/adblockplus/android/...
src/org/adblockplus/android/ProxyService.java:616: if (ourProxy !=
proxyManualyConfigured)
rename to proxyManuallyConfigured

http://codereview.adblockplus.org/9423010/diff/1/src/org/adblockplus/android/...
src/org/adblockplus/android/ProxyService.java:628: notrafficHandler = new
Handler();
variable name should be camel-cased (otherwise it is inconsistent with
stopNoTrafficCheck and updateNoTrafficCheck)

http://codereview.adblockplus.org/9423010/diff/1/src/org/adblockplus/android/...
src/org/adblockplus/android/ProxyService.java:659: Log.e(TAG, "V: " +
nativeProxy + " " + proxyManualyConfigured + " " + transparent);
Why is this being logged with priority ERROR?

http://codereview.adblockplus.org/9423010/diff/1/src/org/adblockplus/android/...
src/org/adblockplus/android/ProxyService.java:665: //builder.setContent(new
RemoteViews(getPackageName(), R.layout.notif_hidden));
you can add a TODO here but keep unused code in local repo

Powered by Google App Engine
This is Rietveld