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

Issue 6216090891845632: Issue #404 - Create common library shared between plugin/engine and installer (Closed)

Created:
Jan. 9, 2015, 8:07 p.m. by Eric
Modified:
May 19, 2015, 7:04 p.m.
Reviewers:
sergei, Oleksandr
CC:
Felix Dahlke
Visibility:
Public.

Description

Issue #404 - Create common library shared between plugin/engine and installer Create new static library 'common' as a separate project with its own directory. It uses a separate .gypi include file so that it can be included in both the plugin/engine solution and in the installer solution. This change set does not incorporate changes to the installer, because it's not using these modules yet. Contents of the library are the registry and IE version classes. Split out the unit tests for these two modules into separate source files. Refactor the unit tests to conform to naming conventions. Add one unit test for Registry.

Patch Set 1 #

Total comments: 2

Patch Set 2 : Rename to IeVersion*.*; fix URL in copyright notice #

Total comments: 17

Patch Set 3 : changed spacing; fixed typo #

Patch Set 4 : final check after rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+256 lines, -180 lines) Patch
M adblockplus.gyp View 1 2 3 5 chunks +10 lines, -9 lines 0 comments Download
A common/common.gypi View 1 2 1 chunk +59 lines, -0 lines 0 comments Download
M common/include/IeVersion.h View 1 0 chunks +-1 lines, --1 lines 0 comments Download
M common/include/Registry.h View 0 chunks +-1 lines, --1 lines 0 comments Download
M common/src/IeVersion.cpp View 1 1 chunk +95 lines, -96 lines 0 comments Download
M common/src/Registry.cpp View 0 chunks +-1 lines, --1 lines 0 comments Download
A common/test/IeVersionTest.cpp View 1 2 1 chunk +65 lines, -0 lines 0 comments Download
M common/test/RegistryTest.cpp View 1 1 chunk +26 lines, -74 lines 0 comments Download
M src/engine/Main.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
M src/plugin/PluginClass.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
M src/plugin/PluginTabBase.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
M src/plugin/PluginWbPassThrough.cpp View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 20
Eric
Jan. 9, 2015, 8:10 p.m. (2015-01-09 20:10:22 UTC) #1
Oleksandr
* I am not convinced we need yet another shared project. Why can't we just ...
Feb. 4, 2015, 10 a.m. (2015-02-04 10:00:41 UTC) #2
Eric
On 2015/02/04 10:00:41, Oleksandr wrote: > * I am not convinced we need yet another ...
Feb. 4, 2015, 4:13 p.m. (2015-02-04 16:13:02 UTC) #3
Oleksandr
On 2015/02/04 16:13:02, Eric wrote: > The scope is sharing is different. The current share ...
Feb. 4, 2015, 9:45 p.m. (2015-02-04 21:45:31 UTC) #4
sergei
I have not check the changes in details yet. On 2015/02/04 21:45:31, Oleksandr wrote: > ...
Feb. 5, 2015, 1:18 p.m. (2015-02-05 13:18:21 UTC) #5
Eric
On 2015/02/04 21:45:31, Oleksandr wrote: > Still, I would just use the shared project in ...
Feb. 11, 2015, 5:07 p.m. (2015-02-11 17:07:41 UTC) #6
Oleksandr
It seems that we both of you agree that we need a separate project. I ...
Feb. 27, 2015, 6:06 a.m. (2015-02-27 06:06:19 UTC) #7
Eric
Patch set two has the changes Oleksandr asked for. http://codereview.adblockplus.org/6216090891845632/diff/5629499534213120/common/common.gypi File common/common.gypi (right): http://codereview.adblockplus.org/6216090891845632/diff/5629499534213120/common/common.gypi#newcode1 common/common.gypi:1: ...
Feb. 27, 2015, 1:55 p.m. (2015-02-27 13:55:10 UTC) #8
Oleksandr
Just 2 more comments http://codereview.adblockplus.org/6216090891845632/diff/5676830073815040/common/common.gypi File common/common.gypi (right): http://codereview.adblockplus.org/6216090891845632/diff/5676830073815040/common/common.gypi#newcode53 common/common.gypi:53: 'SubSystem': '1', # Console Nit: ...
March 4, 2015, 5:26 a.m. (2015-03-04 05:26:29 UTC) #9
sergei
http://codereview.adblockplus.org/6216090891845632/diff/5676830073815040/common/common.gypi File common/common.gypi (right): http://codereview.adblockplus.org/6216090891845632/diff/5676830073815040/common/common.gypi#newcode53 common/common.gypi:53: 'SubSystem': '1', # Console On 2015/03/04 05:26:29, Oleksandr wrote: ...
March 4, 2015, 8:02 a.m. (2015-03-04 08:02:00 UTC) #10
Eric
http://codereview.adblockplus.org/6216090891845632/diff/5676830073815040/common/common.gypi File common/common.gypi (right): http://codereview.adblockplus.org/6216090891845632/diff/5676830073815040/common/common.gypi#newcode53 common/common.gypi:53: 'SubSystem': '1', # Console On 2015/03/04 05:26:29, Oleksandr wrote: ...
March 4, 2015, 2:15 p.m. (2015-03-04 14:15:16 UTC) #11
Oleksandr
http://codereview.adblockplus.org/6216090891845632/diff/5676830073815040/common/common.gypi File common/common.gypi (right): http://codereview.adblockplus.org/6216090891845632/diff/5676830073815040/common/common.gypi#newcode53 common/common.gypi:53: 'SubSystem': '1', # Console Yes, I actually was meant ...
March 4, 2015, 3:37 p.m. (2015-03-04 15:37:23 UTC) #12
sergei
http://codereview.adblockplus.org/6216090891845632/diff/5676830073815040/common/test/IeVersionTest.cpp File common/test/IeVersionTest.cpp (right): http://codereview.adblockplus.org/6216090891845632/diff/5676830073815040/common/test/IeVersionTest.cpp#newcode23 common/test/IeVersionTest.cpp:23: * If they are disabled, gtext will report 2 ...
March 4, 2015, 3:53 p.m. (2015-03-04 15:53:55 UTC) #13
Eric
New patch set. http://codereview.adblockplus.org/6216090891845632/diff/5676830073815040/common/common.gypi File common/common.gypi (right): http://codereview.adblockplus.org/6216090891845632/diff/5676830073815040/common/common.gypi#newcode53 common/common.gypi:53: 'SubSystem': '1', # Console On 2015/03/04 ...
March 4, 2015, 4:55 p.m. (2015-03-04 16:55:10 UTC) #14
Felix Dahlke
http://codereview.adblockplus.org/6216090891845632/diff/5676830073815040/common/test/IeVersionTest.cpp File common/test/IeVersionTest.cpp (right): http://codereview.adblockplus.org/6216090891845632/diff/5676830073815040/common/test/IeVersionTest.cpp#newcode25 common/test/IeVersionTest.cpp:25: #if !defined(DISABLE_EXACT_TESTS) > If we change it, we can ...
March 5, 2015, 1:33 p.m. (2015-03-05 13:33:06 UTC) #15
Eric
http://codereview.adblockplus.org/6216090891845632/diff/5676830073815040/common/test/IeVersionTest.cpp File common/test/IeVersionTest.cpp (right): http://codereview.adblockplus.org/6216090891845632/diff/5676830073815040/common/test/IeVersionTest.cpp#newcode25 common/test/IeVersionTest.cpp:25: #if !defined(DISABLE_EXACT_TESTS) On 2015/03/05 13:33:06, Felix H. Dahlke wrote: ...
March 5, 2015, 2:49 p.m. (2015-03-05 14:49:48 UTC) #16
Eric
Let's try that again. On 2015/03/05 13:33:06, Felix H. Dahlke wrote: > The code is ...
March 5, 2015, 2:53 p.m. (2015-03-05 14:53:05 UTC) #17
Felix Dahlke
On 2015/03/05 14:53:05, Eric wrote: > Let's try that again. > > On 2015/03/05 13:33:06, ...
March 5, 2015, 2:59 p.m. (2015-03-05 14:59:17 UTC) #18
Oleksandr
LGTM
March 6, 2015, 3:23 a.m. (2015-03-06 03:23:51 UTC) #19
sergei
April 2, 2015, 7:46 a.m. (2015-04-02 07:46:18 UTC) #20
LGTM

Powered by Google App Engine
This is Rietveld