Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(210)

Issue 29398655: Issue 5062 - [emscripten] Allow generation of custom bindings code (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 6 months ago by Wladimir Palant
Modified:
2 years, 6 months ago
Reviewers:
sergei, hub
CC:
Felix Dahlke
Base URL:
https://hg.adblockplus.org/adblockpluscore
Visibility:
Public.

Description

Issue 5062 - [emscripten] Allow generation of custom bindings code

Patch Set 1 #

Total comments: 26

Patch Set 2 : Improved initialization code #

Total comments: 4

Patch Set 3 : Made the code slightly safer #

Total comments: 2

Patch Set 4 : Improved binding generation #

Patch Set 5 : Better variable name #

Total comments: 2

Patch Set 6 : Fixed mistakenly exported function #

Patch Set 7 : Rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+63 lines, -68 lines) Patch
M README.md View 1 chunk +12 lines, -17 lines 0 comments Download
M compile View 2 chunks +2 lines, -4 lines 0 comments Download
M compiled/bindings.cpp View 1 chunk +3 lines, -2 lines 0 comments Download
M compiled/bindings.ipp View 1 2 3 4 5 6 6 chunks +31 lines, -40 lines 0 comments Download
M compiled/filter/RegExpFilter.h View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M compiled/filter/RegExpFilter.cpp View 1 2 3 4 2 chunks +14 lines, -4 lines 0 comments Download

Messages

Total messages: 14
Wladimir Palant
2 years, 6 months ago (2017-03-30 07:59:29 UTC) #1
sergei
I have not tried it, but in general looks good, only a couple of moments. ...
2 years, 6 months ago (2017-03-30 11:09:18 UTC) #2
Wladimir Palant
https://codereview.adblockplus.org/29398655/diff/29398656/README.md File README.md (left): https://codereview.adblockplus.org/29398655/diff/29398656/README.md#oldcode22 README.md:22: * [Node.js 6 or higher](https://nodejs.org/en/) On 2017/03/30 11:09:17, sergei ...
2 years, 6 months ago (2017-03-30 12:58:58 UTC) #3
Wladimir Palant
Added Hubert to CC.
2 years, 6 months ago (2017-03-30 17:41:03 UTC) #4
hub
https://codereview.adblockplus.org/29398655/diff/29398712/compiled/filter/RegExpFilter.cpp File compiled/filter/RegExpFilter.cpp (right): https://codereview.adblockplus.org/29398655/diff/29398712/compiled/filter/RegExpFilter.cpp#newcode353 compiled/filter/RegExpFilter.cpp:353: for (auto it = typeMap.begin(); it != typeMap.end(); ++it) ...
2 years, 6 months ago (2017-03-31 02:57:22 UTC) #5
Wladimir Palant
https://codereview.adblockplus.org/29398655/diff/29398712/compiled/filter/RegExpFilter.cpp File compiled/filter/RegExpFilter.cpp (right): https://codereview.adblockplus.org/29398655/diff/29398712/compiled/filter/RegExpFilter.cpp#newcode353 compiled/filter/RegExpFilter.cpp:353: for (auto it = typeMap.begin(); it != typeMap.end(); ++it) ...
2 years, 6 months ago (2017-04-04 14:26:01 UTC) #6
sergei
https://codereview.adblockplus.org/29398655/diff/29398656/README.md File README.md (left): https://codereview.adblockplus.org/29398655/diff/29398656/README.md#oldcode22 README.md:22: * [Node.js 6 or higher](https://nodejs.org/en/) On 2017/03/30 12:58:57, Wladimir ...
2 years, 6 months ago (2017-04-04 14:49:31 UTC) #7
Wladimir Palant
https://codereview.adblockplus.org/29398655/diff/29398656/compiled/bindings.ipp File compiled/bindings.ipp (right): https://codereview.adblockplus.org/29398655/diff/29398656/compiled/bindings.ipp#newcode621 compiled/bindings.ipp:621: int main(void)\ On 2017/04/04 14:49:30, sergei wrote: > I ...
2 years, 6 months ago (2017-04-04 15:41:49 UTC) #8
Wladimir Palant
In Patch set 5 I changed the loop variable name to item which is consistent ...
2 years, 6 months ago (2017-04-06 08:13:07 UTC) #9
hub
LGTM
2 years, 6 months ago (2017-04-11 09:17:21 UTC) #10
sergei
LGTM https://codereview.adblockplus.org/29398655/diff/29398656/compiled/bindings.ipp File compiled/bindings.ipp (right): https://codereview.adblockplus.org/29398655/diff/29398656/compiled/bindings.ipp#newcode621 compiled/bindings.ipp:621: int main(void)\ On 2017/04/04 15:41:48, Wladimir Palant wrote: ...
2 years, 6 months ago (2017-04-11 16:29:20 UTC) #11
Wladimir Palant
Patch set 6 removes bogus EMSCRIPTEN_KEEPALIVE. Other than that it is only rebasing changes. https://codereview.adblockplus.org/29398655/diff/29398656/compiled/bindings.ipp ...
2 years, 6 months ago (2017-04-11 18:22:44 UTC) #12
sergei
On 2017/04/11 18:22:44, Wladimir Palant wrote: > Patch set 6 removes bogus EMSCRIPTEN_KEEPALIVE. Other than ...
2 years, 6 months ago (2017-04-18 08:07:41 UTC) #13
hub
2 years, 6 months ago (2017-04-18 11:45:28 UTC) #14
Message was sent while issue was closed.
LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5