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

Issue 5979045791531008: create utils project with shared files (Closed)

Created:
July 16, 2014, 12:23 p.m. by sergei
Modified:
July 17, 2014, 10:08 a.m.
Reviewers:
Felix Dahlke, Oleksandr
Visibility:
Public.

Description

https://issues.adblockplus.org/ticket/1050 create utils project with shared files Previously they were included into each project. Some benefits from it: - DRY, it's already used by AdblockPlus, AdblockplusEngine and by tests - Less compilation time. - Theoretically it can happen that in tests there are some defines which change the code. Linking the same library we have the tested code. - No repeating - less disorder. - Also one have to be sure that it's compilable in all projects instead of in only one.

Patch Set 1 #

Total comments: 2

Patch Set 2 : rename project name "utils"->"shared" #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -4 lines) Patch
M adblockplus.gyp View 1 4 chunks +4 lines, -4 lines 0 comments Download

Messages

Total messages: 7
sergei
July 16, 2014, 12:25 p.m. (2014-07-16 12:25:38 UTC) #1
Felix Dahlke
I agree, makes a lot of sense. Just one comment. http://codereview.adblockplus.org/5979045791531008/diff/5629499534213120/adblockplus.gyp File adblockplus.gyp (right): http://codereview.adblockplus.org/5979045791531008/diff/5629499534213120/adblockplus.gyp#newcode34 ...
July 16, 2014, 3:17 p.m. (2014-07-16 15:17:19 UTC) #2
Oleksandr
LGTM On 2014/07/16 15:17:19, Felix H. Dahlke wrote: > I agree, makes a lot of ...
July 16, 2014, 3:28 p.m. (2014-07-16 15:28:06 UTC) #3
sergei
http://codereview.adblockplus.org/5979045791531008/diff/5629499534213120/adblockplus.gyp File adblockplus.gyp (right): http://codereview.adblockplus.org/5979045791531008/diff/5629499534213120/adblockplus.gyp#newcode34 adblockplus.gyp:34: 'target_name': 'utils', On 2014/07/16 15:17:19, Felix H. Dahlke wrote: ...
July 16, 2014, 3:50 p.m. (2014-07-16 15:50:17 UTC) #4
Felix Dahlke
On 2014/07/16 15:50:17, sergei wrote: > http://codereview.adblockplus.org/5979045791531008/diff/5629499534213120/adblockplus.gyp > File adblockplus.gyp (right): > > http://codereview.adblockplus.org/5979045791531008/diff/5629499534213120/adblockplus.gyp#newcode34 > ...
July 16, 2014, 4:12 p.m. (2014-07-16 16:12:28 UTC) #5
sergei
July 16, 2014, 4:30 p.m. (2014-07-16 16:30:53 UTC) #6
Felix Dahlke
July 16, 2014, 4:36 p.m. (2014-07-16 16:36:44 UTC) #7
LGTM!

Powered by Google App Engine
This is Rietveld