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

Issue 10802049: Functional prefs implementation (Closed)

Created:
June 5, 2013, 9:03 a.m. by Wladimir Palant
Modified:
June 5, 2013, 12:14 p.m.
Reviewers:
Felix Dahlke, Oleksandr
Visibility:
Public.

Description

Functional prefs implementation

Patch Set 1 #

Patch Set 2 : Cleaned up init.js a bit #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+439 lines, -16 lines) Patch
M include/AdblockPlus/FilterEngine.h View 1 chunk +4 lines, -1 line 1 comment Download
M lib/api.js View 2 chunks +11 lines, -0 lines 0 comments Download
M lib/init.js View 1 2 chunks +24 lines, -2 lines 0 comments Download
M lib/prefs.js View 1 chunk +93 lines, -2 lines 0 comments Download
M shell/shell.gyp View 1 chunk +2 lines, -1 line 0 comments Download
M shell/src/Main.cpp View 2 chunks +3 lines, -1 line 0 comments Download
A shell/src/PrefsCommand.h View 1 chunk +38 lines, -0 lines 0 comments Download
A shell/src/PrefsCommand.cpp View 1 chunk +101 lines, -0 lines 0 comments Download
M src/FilterEngine.cpp View 4 chunks +27 lines, -3 lines 0 comments Download
M test/BaseJsTest.h View 2 chunks +8 lines, -6 lines 0 comments Download
M test/FilterEngine.cpp View 2 chunks +128 lines, -0 lines 3 comments Download

Messages

Total messages: 4
Wladimir Palant
June 5, 2013, 9:03 a.m. (2013-06-05 09:03:24 UTC) #1
Wladimir Palant
June 5, 2013, 9:44 a.m. (2013-06-05 09:44:10 UTC) #2
Felix Dahlke
LGTM with some nits http://codereview.adblockplus.org/10802049/diff/3001/include/AdblockPlus/FilterEngine.h File include/AdblockPlus/FilterEngine.h (right): http://codereview.adblockplus.org/10802049/diff/3001/include/AdblockPlus/FilterEngine.h#newcode71 include/AdblockPlus/FilterEngine.h:71: JsEnginePtr GetJsEngine() const {return jsEngine; ...
June 5, 2013, 10:50 a.m. (2013-06-05 10:50:52 UTC) #3
Wladimir Palant
June 5, 2013, 12:14 p.m. (2013-06-05 12:14:06 UTC) #4
The prefs test got some more extensive refactoring in
https://hg.adblockplus.org/libadblockplus/rev/e68e9306f096 (moved into a
separate file with its own base type), otherwise all comments have been fixed as
suggested.

Powered by Google App Engine
This is Rietveld