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

Issue 5469643829411840: Issue 1795 - Add helper std::wstring GetLocationUrl(IWebBrowser2& browser) (Closed)

Created:
Jan. 13, 2015, 3:22 p.m. by sergei
Modified:
Feb. 5, 2015, 10:35 a.m.
Reviewers:
Eric, Oleksandr
Visibility:
Public.

Patch Set 1 #

Total comments: 8

Patch Set 2 : get rid of variable in the function body #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+46 lines, -34 lines) Patch
M src/plugin/PluginUtil.h View 1 2 chunks +19 lines, -17 lines 1 comment Download
M src/plugin/PluginUtil.cpp View 1 2 chunks +27 lines, -17 lines 0 comments Download

Messages

Total messages: 6
sergei
Jan. 13, 2015, 3:25 p.m. (2015-01-13 15:25:28 UTC) #1
Eric
http://codereview.adblockplus.org/5469643829411840/diff/5629499534213120/src/plugin/PluginUtil.cpp File src/plugin/PluginUtil.cpp (right): http://codereview.adblockplus.org/5469643829411840/diff/5629499534213120/src/plugin/PluginUtil.cpp#newcode34 src/plugin/PluginUtil.cpp:34: std::wstring retValue; Use of this variable requires extraneous assignments. ...
Jan. 13, 2015, 5:04 p.m. (2015-01-13 17:04:20 UTC) #2
sergei
http://codereview.adblockplus.org/5469643829411840/diff/5629499534213120/src/plugin/PluginUtil.cpp File src/plugin/PluginUtil.cpp (right): http://codereview.adblockplus.org/5469643829411840/diff/5629499534213120/src/plugin/PluginUtil.cpp#newcode34 src/plugin/PluginUtil.cpp:34: std::wstring retValue; On 2015/01/13 17:04:20, Eric wrote: > Use ...
Jan. 29, 2015, 12:42 p.m. (2015-01-29 12:42:11 UTC) #3
Oleksandr
LGTM http://codereview.adblockplus.org/5469643829411840/diff/5685265389584384/src/plugin/PluginUtil.h File src/plugin/PluginUtil.h (right): http://codereview.adblockplus.org/5469643829411840/diff/5685265389584384/src/plugin/PluginUtil.h#newcode26 src/plugin/PluginUtil.h:26: No new string in the end of file, ...
Jan. 30, 2015, 2:29 p.m. (2015-01-30 14:29:45 UTC) #4
Eric
LGTM
Feb. 2, 2015, 6:44 p.m. (2015-02-02 18:44:53 UTC) #5
Eric
Feb. 2, 2015, 7:07 p.m. (2015-02-02 19:07:54 UTC) #6
http://codereview.adblockplus.org/5469643829411840/diff/5629499534213120/src/...
File src/plugin/PluginUtil.h (right):

http://codereview.adblockplus.org/5469643829411840/diff/5629499534213120/src/...
src/plugin/PluginUtil.h:8: std::wstring GetLocationUrl(IWebBrowser2& browser);
On 2015/01/29 12:42:11, sergei wrote:
> Should I create the class for it?

The next time you write a function with an argument of type 'IBrowser2&', that's
the time to create a class for it and use methods. Not so much need when there's
only one loose function about.

Powered by Google App Engine
This is Rietveld