Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(89)

Issue 320743002: Add unittest for 'AR' in 'make_global_settings' (Closed)

Created:
6 years, 6 months ago by yukawa
Modified:
6 years, 6 months ago
Reviewers:
Torne, scottmg
CC:
gyp-developer_googlegroups.com
Base URL:
https://chromium.googlesource.com/external/gyp.git@master
Visibility:
Public.

Description

Add unittest for 'AR' in 'make_global_settings' Currently sharing the code among host/NaCl/NDK/Emscripten on top of Ninja+gyp isn't so easy as it should be because not all items in 'make_global_settings' are recognized by Ninja generator (gyp:434). On the other hand, the current test coverage of 'make_global_settings' isn't so good. Thus this CL describes the current behavior with some unittests before making Ninja generator aware of 'AR' in 'make_global_settings' like Make generator. BUG=gyp:434 TEST=unittest R=scottmg@chromium.org Committed: https://code.google.com/p/gyp/source/detail?r=1931

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+163 lines, -0 lines) Patch
A test/make_global_settings/ar/gyptest-make_global_settings_ar.py View 1 chunk +134 lines, -0 lines 0 comments Download
A test/make_global_settings/ar/make_global_settings_ar.gyp View 1 chunk +29 lines, -0 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
yukawa
Hey Scott, Can you take a look at this CL? Though this CL never changes ...
6 years, 6 months ago (2014-06-08 18:12:38 UTC) #1
scottmg
This lgtm, but adding Torne and Ami who have expressed more interest/know more about .host ...
6 years, 6 months ago (2014-06-09 16:02:37 UTC) #2
Torne
As long as this is consistent with the other settings this should be fine.
6 years, 6 months ago (2014-06-09 16:06:38 UTC) #3
yukawa
On 2014/06/09 16:06:38, Torne wrote: > As long as this is consistent with the other ...
6 years, 6 months ago (2014-06-09 16:37:50 UTC) #4
Ami GONE FROM CHROMIUM
I think the stated goal is a good one. Leaving actual review to others.
6 years, 6 months ago (2014-06-09 16:44:03 UTC) #5
yukawa
6 years, 6 months ago (2014-06-09 17:11:29 UTC) #6
Message was sent while issue was closed.
Committed patchset #1 manually as r1931 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698