Taking advantage of new configuration inheritance option....

Taking advantage of new configuration inheritance option.

BUG=None
TEST=None

[M-A Ruel, 2009-10-29 19:10:48]

http://codereview.chromium.org/342041/diff/1/3
File build/common.gypi (right):

http://codereview.chromium.org/342041/diff/1/3#newcode363
Line 363: 'inherit_from': ['Common'],
I assumed this would be implicit? If 'Common' would be a special configuration.
I wouldn't want to see a 'Common' config in VS.

[bradn, 2009-10-29 19:24:58]

Since Common is 'abstract' it won't be present in VS.
Its just there so we've got a place we can insert settings that affect all
configurations.

-BradN

On Thu, Oct 29, 2009 at 12:10 PM, <maruel@chromium.org> wrote:

>
> http://codereview.chromium.org/342041/diff/1/3
> File build/common.gypi (right):
>
> http://codereview.chromium.org/342041/diff/1/3#newcode363
> Line 363: 'inherit_from': ['Common'],
> I assumed this would be implicit? If 'Common' would be a special
> configuration. I wouldn't want to see a 'Common' config in VS.
>
>
> http://codereview.chromium.org/342041
>

[M-A Ruel, 2009-10-29 19:27:03]

Ok, I missed the 'abstract': 1 line.

lgtm

[sgk, 2009-10-29 19:38:28]

http://codereview.chromium.org/342041/diff/1/3
File build/common.gypi (right):

http://codereview.chromium.org/342041/diff/1/3#newcode355
Line 355: 'configuration_platform': 'Win32',
I'd either expect this to be in an OS=="win" conditions block within this
configuration, or else change the name of the field to
'msvs_configuration_platform'.

http://codereview.chromium.org/342041/diff/1/3#newcode409
Line 409: 'msvs_settings': {
For consistency with your change above, move this outside the conditions block,
too.

http://codereview.chromium.org/342041/diff/1/3#newcode444
Line 444: 'inherit_from': ['Release'],
Nice.  Really cleans up the no tcmalloc case.

http://codereview.chromium.org/342041/diff/7001/7003#newcode353
Line 353: 'Common': {
The name "Common" doesn't quite communicate enough to me.  When I looked at it
being added to in tcmalloc.gyp, it wasn't clear that it was adding things to
something that was then going to propagate to all of the other targets' configs.

"Base" alone is too much like "base" the target.  How about "BaseConfiguration"
as the name?  If that works, we might even adopt a naming convention of the
*Configuration suffix implying abstract bases that have "ripple effects" when
they're added to...?

[bradn, 2009-10-29 19:42:13]

http://codereview.chromium.org/342041/diff/1/3
File build/common.gypi (right):

http://codereview.chromium.org/342041/diff/1/3#newcode355
Line 355: 'configuration_platform': 'Win32',
On 2009/10/29 19:38:28, sgk wrote:
> I'd either expect this to be in an OS=="win" conditions block within this
> configuration, or else change the name of the field to
> 'msvs_configuration_platform'.

Done.

http://codereview.chromium.org/342041/diff/1/3#newcode409
Line 409: 'msvs_settings': {
On 2009/10/29 19:38:28, sgk wrote:
> For consistency with your change above, move this outside the conditions
block,
> too.

Done.