remoting/chromoting_test.gyp - Issue 1164143002: Added new driver for chromoting.

Side by Side Diff

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Side by Side Diff: remoting/chromoting_test.gyp

Issue 1164143002: Added new driver for chromoting. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Changed comments to be clearer and alphabetized targets. Created 5 years, 6 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
(Empty)
	1 # Copyright 2015 The Chromium Authors. All rights reserved.
	Sergey Ulanov 2015/06/08 18:57:37 Do we really need to add this extra .gyp file? Why Do we really need to add this extra .gyp file? Why not just add the new target in remoting_test.gypi? I also think app_remoting_test.gyp should be replaced with .gypi or just moved inside remoting_test.gypi. In general there is a cost to each extra .gyp file - they slow down gyp processing. So it's better to avoid adding new .gyp files when possible. Also it requires some code duplication (e.g. include statement), and not consistent with other targets we have in remoting/ . tonychun 2015/06/08 20:35:58 I completely agree. This change would optimize gyp Show quoted text On 2015/06/08 18:57:37, Sergey Ulanov wrote: > Do we really need to add this extra .gyp file? Why not just add the new target > in remoting_test.gypi? > > I also think app_remoting_test.gyp should be replaced with .gypi or just moved > inside remoting_test.gypi. > > In general there is a cost to each extra .gyp file - they slow down gyp > processing. So it's better to avoid adding new .gyp files when possible. > Also it requires some code duplication (e.g. include statement), and not > consistent with other targets we have in remoting/ . I completely agree. This change would optimize gyp processing time. However, I would like to take care of this issue in a separate CL because the objective of this CL is to get a driver up and running with how I refactored the .gyp files. I will have the next CL focused on making that change.
	2 # Use of this source code is governed by a BSD-style license that can be

	3 # found in the LICENSE file.

	4

	5 {

	6 'includes': [

	7 'remoting_version.gypi',

	8 ],

	9

	10 'target_defaults': {
	Sergey Ulanov 2015/06/08 18:57:37 Why do you need this? Why do you need this? tonychun 2015/06/08 20:35:58 I thought, by default, every .gyp file requires a Show quoted text On 2015/06/08 18:57:37, Sergey Ulanov wrote: > Why do you need this? I thought, by default, every .gyp file requires a target_defaults key. It seems like this is not the case, so I can remove it.
	11 'type': 'none',

	12 }, # target_defaults

	13 'targets': [

	14 {

	15 # A chromoting version of the common test driver tool
	joedow 2015/06/08 18:35:20 what is the common test driver tool? what is the common test driver tool? tonychun 2015/06/08 20:35:58 It is the common lib in remoting_all.gyp. I will r Show quoted text On 2015/06/08 18:35:20, joedow wrote: > what is the common test driver tool? It is the common lib in remoting_all.gyp. I will restate the comment to make this more clear.
	16 'target_name': 'chromoting_test_driver',

	17 'type': '<(gtest_target_type)',

	18 'dependencies': [

	19 '<(DEPTH)/remoting/remoting_test_driver_common.gyp:remoting_test_driver_ common',

	20 ],

	21 'defines': [

	22 'VERSION=<(version_full)',

	23 ],

	24 'sources': [

	25 'test/chromoting_test_driver.cc',

	26 ],

	27 'include_dirs': [

	28 '../testing/gtest/include',

	29 ],

	30 }, # end of target 'chromoting_test_driver'

	31 ], # end of targets

	32 }

OLD	NEW

« remoting/app_remoting_test.gyp ('K') | « remoting/app_remoting_test.gyp ('k') | remoting/remoting_all.gyp » ('j') | remoting/remoting_test_driver_common.gyp » ('J')