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

Issue 29385742: Issue 4127 - [emscripten] Convert subscription classes to C++ - Part 2 (Closed)

Created:
March 16, 2017, 6:27 p.m. by Wladimir Palant
Modified:
April 13, 2017, 1:50 p.m.
Reviewers:
sergei, hub
CC:
Felix Dahlke
Base URL:
https://hg.adblockplus.org/adblockpluscore
Visibility:
Public.

Description

Issue 4127 - [emscripten] Convert subscription classes to C++ - Part 2

Patch Set 1 #

Total comments: 4

Patch Set 2 : Use iterators syntax #

Patch Set 3 : Rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+169 lines, -20 lines) Patch
M compiled/bindings.cpp View 1 1 chunk +5 lines, -0 lines 0 comments Download
M compiled/bindings.ipp View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M compiled/filter/Filter.h View 1 2 1 chunk +0 lines, -4 lines 0 comments Download
M compiled/filter/Filter.cpp View 1 1 chunk +8 lines, -8 lines 0 comments Download
M compiled/intrusive_ptr.h View 1 1 chunk +3 lines, -3 lines 0 comments Download
M compiled/subscription/Subscription.h View 1 2 2 chunks +8 lines, -1 line 0 comments Download
M compiled/subscription/Subscription.cpp View 1 2 3 chunks +30 lines, -4 lines 0 comments Download
M compiled/subscription/UserDefinedSubscription.h View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M compiled/subscription/UserDefinedSubscription.cpp View 1 2 1 chunk +17 lines, -0 lines 0 comments Download
M test/subscriptionClasses.js View 1 2 1 chunk +94 lines, -0 lines 0 comments Download

Messages

Total messages: 7
Wladimir Palant
March 16, 2017, 6:27 p.m. (2017-03-16 18:27:08 UTC) #1
Wladimir Palant
https://codereview.adblockplus.org/29385742/diff/29385743/compiled/bindings.ipp File compiled/bindings.ipp (right): https://codereview.adblockplus.org/29385742/diff/29385743/compiled/bindings.ipp#newcode445 compiled/bindings.ipp:445: result += " result = null;\n"; This isn't strictly ...
March 16, 2017, 6:31 p.m. (2017-03-16 18:31:23 UTC) #2
Wladimir Palant
Added Hubert to CC.
March 30, 2017, 5:40 p.m. (2017-03-30 17:40:15 UTC) #3
Wladimir Palant
Patch set 2 introduced lots of rebasing changes unfortunately. The only real change here is ...
April 6, 2017, 8:07 a.m. (2017-04-06 08:07:53 UTC) #4
hub
LGTM
April 6, 2017, 10:06 a.m. (2017-04-06 10:06:24 UTC) #5
sergei
LGTM
April 12, 2017, 5:15 p.m. (2017-04-12 17:15:29 UTC) #6
sergei
April 13, 2017, 1:38 p.m. (2017-04-13 13:38:08 UTC) #7
LGTM

Powered by Google App Engine
This is Rietveld