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

Issue 5712621990838272: [IE] Replace ATL::CSimpleArray (Closed)

Created:
July 20, 2014, 5:36 p.m. by Eric
Modified:
July 26, 2014, 3:13 p.m.
Visibility:
Public.

Description

Replace ATL:CSimpleArray with std::set. This eliminates CSimpleArray

Patch Set 1 #

Total comments: 9

Patch Set 2 : Formatting changes #

Patch Set 3 : full patch #

Total comments: 2

Patch Set 4 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -15 lines) Patch
M src/plugin/PluginClass.h View 2 2 chunks +2 lines, -1 line 0 comments Download
M src/plugin/PluginClass.cpp View 1 2 3 6 chunks +13 lines, -14 lines 0 comments Download

Messages

Total messages: 12
Eric
July 20, 2014, 5:38 p.m. (2014-07-20 17:38:56 UTC) #1
sergei
The first string of the description is neither according to the format "Issue 123 - ...
July 21, 2014, 8:49 a.m. (2014-07-21 08:49:37 UTC) #2
sergei
http://codereview.adblockplus.org/5712621990838272/diff/5629499534213120/src/plugin/PluginClass.cpp File src/plugin/PluginClass.cpp (right): http://codereview.adblockplus.org/5712621990838272/diff/5629499534213120/src/plugin/PluginClass.cpp#newcode1065 src/plugin/PluginClass.cpp:1065: CPluginClass* return_instance = NULL; According to the coding style ...
July 21, 2014, 8:49 a.m. (2014-07-21 08:49:51 UTC) #3
Oleksandr
http://codereview.adblockplus.org/5712621990838272/diff/5629499534213120/src/plugin/PluginClass.cpp File src/plugin/PluginClass.cpp (right): http://codereview.adblockplus.org/5712621990838272/diff/5629499534213120/src/plugin/PluginClass.cpp#newcode264 src/plugin/PluginClass.cpp:264: s_instances.insert( this ); Style nit: no space after '(' ...
July 21, 2014, 9 a.m. (2014-07-21 09:00:54 UTC) #4
sergei
On 2014/07/21 09:00:54, Oleksandr wrote: > http://codereview.adblockplus.org/5712621990838272/diff/5629499534213120/src/plugin/PluginClass.cpp > File src/plugin/PluginClass.cpp (right): > > http://codereview.adblockplus.org/5712621990838272/diff/5629499534213120/src/plugin/PluginClass.cpp#newcode264 > ...
July 21, 2014, 9:41 a.m. (2014-07-21 09:41:53 UTC) #5
Felix Dahlke
On 2014/07/21 09:41:53, sergei wrote: > On 2014/07/21 09:00:54, Oleksandr wrote: > > > http://codereview.adblockplus.org/5712621990838272/diff/5629499534213120/src/plugin/PluginClass.cpp ...
July 21, 2014, 9:47 a.m. (2014-07-21 09:47:26 UTC) #6
Eric
New patch set.
July 22, 2014, 2:29 p.m. (2014-07-22 14:29:43 UTC) #7
sergei
Just, for the future, as I understood, each patch set should be the full ready ...
July 22, 2014, 3:02 p.m. (2014-07-22 15:02:13 UTC) #8
Eric
July 23, 2014, 11:29 a.m. (2014-07-23 11:29:32 UTC) #9
Wladimir Palant
Look good, only one nit below. http://codereview.adblockplus.org/5712621990838272/diff/5733935958982656/src/plugin/PluginClass.cpp File src/plugin/PluginClass.cpp (right): http://codereview.adblockplus.org/5712621990838272/diff/5733935958982656/src/plugin/PluginClass.cpp#newcode1064 src/plugin/PluginClass.cpp:1064: CPluginClass* return_instance = ...
July 24, 2014, 6:41 a.m. (2014-07-24 06:41:53 UTC) #10
Eric
http://codereview.adblockplus.org/5712621990838272/diff/5733935958982656/src/plugin/PluginClass.cpp File src/plugin/PluginClass.cpp (right): http://codereview.adblockplus.org/5712621990838272/diff/5733935958982656/src/plugin/PluginClass.cpp#newcode1064 src/plugin/PluginClass.cpp:1064: CPluginClass* return_instance = nullptr; On 2014/07/24 06:41:53, Wladimir Palant ...
July 24, 2014, 12:33 p.m. (2014-07-24 12:33:47 UTC) #11
Oleksandr
July 25, 2014, 8:37 a.m. (2014-07-25 08:37:08 UTC) #12
LGTM

Powered by Google App Engine
This is Rietveld