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

Issue 29391775: Issue 5013 - Improve some const-correctness (Closed)

Created:
March 22, 2017, 3:37 p.m. by hub
Modified:
March 23, 2017, 1:22 p.m.
Reviewers:
sergei
CC:
Oleksandr, Eric, Felix Dahlke
Base URL:
https://hg.adblockplus.org/libadblockplus/
Visibility:
Public.

Description

Issue 5013 - Improve some const-correctness

Patch Set 1 #

Total comments: 2

Patch Set 2 : Feedback + another small patch related. #

Total comments: 1

Patch Set 3 : Removed GetType() const-ness change #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -29 lines) Patch
M src/DefaultWebRequestCurl.cpp View 1 1 chunk +3 lines, -6 lines 0 comments Download
M src/FilterEngine.cpp View 1 2 3 chunks +6 lines, -9 lines 0 comments Download
M src/JsValue.cpp View 1 2 chunks +4 lines, -4 lines 0 comments Download
M src/Notification.cpp View 1 1 chunk +3 lines, -4 lines 0 comments Download
M src/WebRequestJsObject.cpp View 1 2 chunks +3 lines, -6 lines 0 comments Download

Messages

Total messages: 6
hub
March 22, 2017, 3:37 p.m. (2017-03-22 15:37:27 UTC) #1
hub
Had this patch lying around before you filed the bug. Will run on CI.
March 22, 2017, 3:38 p.m. (2017-03-22 15:38:17 UTC) #2
sergei
What about using of range-for `for(const auto& name : collection)` instead of proposed changes? https://codereview.adblockplus.org/29391775/diff/29391776/src/WebRequestJsObject.cpp ...
March 22, 2017, 3:44 p.m. (2017-03-22 15:44:00 UTC) #3
hub
update patch https://codereview.adblockplus.org/29391775/diff/29391776/src/WebRequestJsObject.cpp File src/WebRequestJsObject.cpp (right): https://codereview.adblockplus.org/29391775/diff/29391776/src/WebRequestJsObject.cpp#newcode46 src/WebRequestJsObject.cpp:46: const std::string & header = *it; On ...
March 22, 2017, 6:16 p.m. (2017-03-22 18:16:59 UTC) #4
sergei
LGTM https://codereview.adblockplus.org/29391775/diff/29391790/include/AdblockPlus/FilterEngine.h File include/AdblockPlus/FilterEngine.h (right): https://codereview.adblockplus.org/29391775/diff/29391790/include/AdblockPlus/FilterEngine.h#newcode55 include/AdblockPlus/FilterEngine.h:55: Type GetType() const; Do you mind to put ...
March 23, 2017, 10:49 a.m. (2017-03-23 10:49:56 UTC) #5
hub
March 23, 2017, 1:22 p.m. (2017-03-23 13:22:49 UTC) #6
Checked in without the GetType() change as requested - patch update here. (later
edited the commit message to be less typo ridden).

Powered by Google App Engine
This is Rietveld