Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(220)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 7 months ago by Eric
Modified:
4 years, 3 months ago
Reviewers:
Oleksandr, sergei
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
4 years, 7 months ago (2015-01-09 20:10:22 UTC) #1
Oleksandr
* I am not convinced we need yet another shared project. Why can't we just ...
4 years, 6 months ago (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 ...
4 years, 6 months ago (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 ...
4 years, 6 months ago (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: > ...
4 years, 6 months ago (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 ...
4 years, 6 months ago (2015-02-11 17:07:41 UTC) #6
Oleksandr
It seems that we both of you agree that we need a separate project. I ...
4 years, 5 months ago (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: ...
4 years, 5 months ago (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: ...
4 years, 5 months ago (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: ...
4 years, 5 months ago (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: ...
4 years, 5 months ago (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 ...
4 years, 5 months ago (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 ...
4 years, 5 months ago (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 ...
4 years, 5 months ago (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 ...
4 years, 5 months ago (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: ...
4 years, 5 months ago (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 ...
4 years, 5 months ago (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, ...
4 years, 5 months ago (2015-03-05 14:59:17 UTC) #18
Oleksandr
LGTM
4 years, 5 months ago (2015-03-06 03:23:51 UTC) #19
sergei
4 years, 4 months ago (2015-04-02 07:46:18 UTC) #20
LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5