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

Issue 29813591: Issue 6526 - Use Maybe<> version of soon to be deprecated API in v8 6.7 (Closed)

Created:
June 22, 2018, 8:54 p.m. by hub
Modified:
Aug. 7, 2018, 5:03 p.m.
Reviewers:
sergei
Base URL:
https://hg.adblockplus.org/libadblockplus/
Visibility:
Public.

Description

Issue 6526 - Use Maybe<> version of soon to be deprecated API in v8 6.7

Patch Set 1 #

Total comments: 2

Patch Set 2 : Rebased #

Total comments: 2

Patch Set 3 : Throw on empty value (AsInt() and As Bool()) #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+45 lines, -15 lines) Patch
M src/JsEngine.cpp View 1 2 chunks +11 lines, -7 lines 0 comments Download
M src/JsValue.cpp View 1 2 2 chunks +15 lines, -5 lines 1 comment Download
M src/Utils.h View 1 2 chunks +17 lines, -1 line 0 comments Download
M test/JsValue.cpp View 1 2 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 6
hub
June 22, 2018, 8:54 p.m. (2018-06-22 20:54:43 UTC) #1
hub
This is the last bits. On top of the previous patch https://codereview.adblockplus.org/29812649/ There are 3 ...
June 22, 2018, 8:55 p.m. (2018-06-22 20:55:48 UTC) #2
hub
https://codereview.adblockplus.org/29813591/diff/29813592/src/JsValue.cpp File src/JsValue.cpp (right): https://codereview.adblockplus.org/29813591/diff/29813592/src/JsValue.cpp#newcode141 src/JsValue.cpp:141: return value.IsJust() ? value.FromJust() : 0; The test expect ...
June 22, 2018, 9:32 p.m. (2018-06-22 21:32:39 UTC) #3
sergei
https://codereview.adblockplus.org/29813591/diff/29813592/test/JsValue.cpp File test/JsValue.cpp (right): https://codereview.adblockplus.org/29813591/diff/29813592/test/JsValue.cpp#newcode214 test/JsValue.cpp:214: TEST_F(JsValueTest, ThrowingConversion) It's unrelated but OK. https://codereview.adblockplus.org/29813591/diff/29848562/src/JsValue.cpp File src/JsValue.cpp ...
Aug. 7, 2018, 1:14 p.m. (2018-08-07 13:14:52 UTC) #4
hub
https://codereview.adblockplus.org/29813591/diff/29848562/src/JsValue.cpp File src/JsValue.cpp (right): https://codereview.adblockplus.org/29813591/diff/29848562/src/JsValue.cpp#newcode141 src/JsValue.cpp:141: return value.IsJust() ? value.FromJust() : 0; On 2018/08/07 13:14:52, ...
Aug. 7, 2018, 2:36 p.m. (2018-08-07 14:36:48 UTC) #5
sergei
Aug. 7, 2018, 4:47 p.m. (2018-08-07 16:47:35 UTC) #6 
LGTM

https://codereview.adblockplus.org/29813591/diff/29849570/src/JsValue.cpp
File src/JsValue.cpp (right):

https://codereview.adblockplus.org/29813591/diff/29849570/src/JsValue.cpp#new...
src/JsValue.cpp:141: return CHECKED_TO_VALUE(std::move(value));
I find the usage of std::move good here too, for the sake of readability.

Powered by Google App Engine
This is Rietveld