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

Issue 10198022: Pass application data into libadblockplus (Closed)

Created:
April 12, 2013, 1:16 p.m. by Felix Dahlke
Modified:
April 18, 2013, 3:24 a.m.
Visibility:
Public.

Description

It looks like this is all the data we need to pass in. I'm not entirely happy with the naming, see my comment below.

Patch Set 1 : #

Total comments: 5

Patch Set 2 : Merged upstream, addressed issues, renamed AppInfo fields #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+159 lines, -62 lines) Patch
M include/AdblockPlus.h View 1 1 chunk +1 line, -0 lines 0 comments Download
A include/AdblockPlus/AppInfo.h View 1 1 chunk +17 lines, -0 lines 0 comments Download
M include/AdblockPlus/JsEngine.h View 1 2 chunks +3 lines, -1 line 0 comments Download
R lib/info.js View 1 1 chunk +4 lines, -4 lines 0 comments Download
M libadblockplus.gyp View 1 2 chunks +2 lines, -0 lines 0 comments Download
M shell/src/Main.cpp View 1 1 chunk +5 lines, -1 line 0 comments Download
A src/AppInfoJsObject.h View 1 chunk +17 lines, -0 lines 0 comments Download
A src/AppInfoJsObject.cpp View 1 1 chunk +20 lines, -0 lines 1 comment Download
M src/GlobalJsObject.h View 1 1 chunk +3 lines, -1 line 0 comments Download
M src/GlobalJsObject.cpp View 1 3 chunks +6 lines, -1 line 0 comments Download
M src/JsEngine.cpp View 1 2 chunks +5 lines, -3 lines 0 comments Download
A test/AppInfoJsObject.cpp View 1 1 chunk +16 lines, -0 lines 0 comments Download
M test/ConsoleJsObject.cpp View 2 chunks +3 lines, -3 lines 0 comments Download
M test/FileSystemJsObject.cpp View 1 15 chunks +15 lines, -15 lines 0 comments Download
M test/FilterEngineStubs.cpp View 1 7 chunks +14 lines, -7 lines 0 comments Download
M test/GlobalJsObject.cpp View 1 3 chunks +4 lines, -4 lines 0 comments Download
M test/JsEngine.cpp View 1 3 chunks +8 lines, -6 lines 0 comments Download
M test/JsValue.cpp View 1 9 chunks +9 lines, -9 lines 0 comments Download
M test/WebRequest.cpp View 1 7 chunks +7 lines, -7 lines 0 comments Download

Messages

Total messages: 6
Felix Dahlke
http://codereview.adblockplus.org/10198022/diff/1020/include/AdblockPlus/AppInfo.h File include/AdblockPlus/AppInfo.h (right): http://codereview.adblockplus.org/10198022/diff/1020/include/AdblockPlus/AppInfo.h#newcode8 include/AdblockPlus/AppInfo.h:8: struct AppInfo I'm not happy with the property names ...
April 12, 2013, 1:32 p.m. (2013-04-12 13:32:27 UTC) #1
Oleksandr
Pretty straightforward. LGTM http://codereview.adblockplus.org/10198022/diff/1020/include/AdblockPlus/AppInfo.h File include/AdblockPlus/AppInfo.h (right): http://codereview.adblockplus.org/10198022/diff/1020/include/AdblockPlus/AppInfo.h#newcode8 include/AdblockPlus/AppInfo.h:8: struct AppInfo Looks fine to me ...
April 12, 2013, 2:14 p.m. (2013-04-12 14:14:52 UTC) #2
Wladimir Palant
http://codereview.adblockplus.org/10198022/diff/1020/include/AdblockPlus/AppInfo.h File include/AdblockPlus/AppInfo.h (right): http://codereview.adblockplus.org/10198022/diff/1020/include/AdblockPlus/AppInfo.h#newcode12 include/AdblockPlus/AppInfo.h:12: std::string addonRoot; addonRoot is probably unnecessary - it is ...
April 15, 2013, 10:56 a.m. (2013-04-15 10:56:00 UTC) #3
Wladimir Palant
http://codereview.adblockplus.org/10198022/diff/1020/lib/info.js File lib/info.js (left): http://codereview.adblockplus.org/10198022/diff/1020/lib/info.js#oldcode1 lib/info.js:1: exports.addonID = "none"; I only realized now that you ...
April 16, 2013, 6:35 a.m. (2013-04-16 06:35:26 UTC) #4
Felix Dahlke
Merged a bunch of upstream changes, addressed the issues found so far and renamed the ...
April 17, 2013, 2:50 p.m. (2013-04-17 14:50:02 UTC) #5
Wladimir Palant
April 17, 2013, 5:53 p.m. (2013-04-17 17:53:24 UTC) #6 
LGTM with the nit fixed.

http://codereview.adblockplus.org/10198022/diff/5002/src/AppInfoJsObject.cpp
File src/AppInfoJsObject.cpp (right):

http://codereview.adblockplus.org/10198022/diff/5002/src/AppInfoJsObject.cpp#...
src/AppInfoJsObject.cpp:12: v8::String::New(appInfo.id.c_str()));
Utils::ToV8String()?

Powered by Google App Engine
This is Rietveld