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

Issue 5733210436665344: Issue #1234 - Remove local CString variable from TraverseDocument() (Closed)

Created:
May 20, 2015, 8:12 p.m. by Eric
Modified:
Oct. 29, 2015, 4:43 p.m.
Reviewers:
sergei, Oleksandr
CC:
Felix Dahlke
Visibility:
Public.

Description

Issue #1234 - Remove local CString variable from TraverseDocument() Add template function BeginsWith() and unit tests for it. Use it to replace local CString variable 'srcLegacy' with std::wstring.

Patch Set 1 : #

Total comments: 1

Patch Set 2 : #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+88 lines, -9 lines) Patch
M adblockplus.gyp View 1 1 chunk +2 lines, -0 lines 0 comments Download
M src/plugin/PluginDomTraverserBase.h View 1 1 chunk +5 lines, -7 lines 0 comments Download
M src/plugin/PluginUtil.h View 1 2 chunks +8 lines, -2 lines 1 comment Download
A test/plugin/UtilTest.cpp View 1 chunk +73 lines, -0 lines 0 comments Download

Messages

Total messages: 8
Eric
May 20, 2015, 8:26 p.m. (2015-05-20 20:26:42 UTC) #1
Sebastian Noack
Did you intentionally put me as reviewer?
May 20, 2015, 8:29 p.m. (2015-05-20 20:29:42 UTC) #2
Eric
On 2015/05/20 20:29:42, Sebastian Noack wrote: > Did you intentionally put me as reviewer? Sorry, ...
May 20, 2015, 8:36 p.m. (2015-05-20 20:36:11 UTC) #3
sergei
https://codereview.adblockplus.org/5733210436665344/diff/5668600916475904/src/plugin/PluginUtil.h File src/plugin/PluginUtil.h (right): https://codereview.adblockplus.org/5733210436665344/diff/5668600916475904/src/plugin/PluginUtil.h#newcode29 src/plugin/PluginUtil.h:29: bool BeginsWith(const std::basic_string<CharT, Traits, Alloc> s, const CharT(&beginning)[N]) reference ...
Oct. 1, 2015, 4:23 p.m. (2015-10-01 16:23:42 UTC) #4
Oleksandr
LGTM, with Sergei's NIT addressed.
Oct. 5, 2015, 10:23 a.m. (2015-10-05 10:23:19 UTC) #5
Eric
New patch set includes rebase to current tip. Actual change fixes Sergei's comment.
Oct. 9, 2015, 3:52 p.m. (2015-10-09 15:52:37 UTC) #6
Oleksandr
LGTM again, just in case. https://codereview.adblockplus.org/5733210436665344/diff/29329021/src/plugin/PluginUtil.h File src/plugin/PluginUtil.h (right): https://codereview.adblockplus.org/5733210436665344/diff/29329021/src/plugin/PluginUtil.h#newcode31 src/plugin/PluginUtil.h:31: return 0 == s.compare(0, ...
Oct. 16, 2015, 8:03 a.m. (2015-10-16 08:03:04 UTC) #7
sergei
Oct. 16, 2015, 8:08 a.m. (2015-10-16 08:08:40 UTC) #8
LGTM

Powered by Google App Engine
This is Rietveld