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

Issue 1254963012: Fix generated file dependencies for upcoming gn (Closed)

Created:
5 years, 4 months ago by cdotstout
Modified:
5 years, 4 months ago
Reviewers:
jamesr, brettw
CC:
Aaron Boodman, abarth-chromium, ben+mojo_chromium.org, darin (slow to review), gregsimon, mojo-reviews_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org
Base URL:
https://github.com/domokit/mojo.git@master
Target Ref:
refs/heads/master
Project:
mojo
Visibility:
Public.

Description

Patch Set 1 #

Total comments: 4

Patch Set 2 : #

Total comments: 2

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+49 lines, -7 lines) Patch
M mojo/public/dart/rules.gni View 1 1 chunk +2 lines, -2 lines 0 comments Download
M mojo/public/mojo_application.gni View 1 chunk +6 lines, -1 line 0 comments Download
M services/python/content_handler/BUILD.gn View 3 chunks +7 lines, -2 lines 0 comments Download
M shell/BUILD.gn View 1 2 5 chunks +25 lines, -2 lines 0 comments Download
M third_party/cython/rules.gni View 1 2 chunks +9 lines, -0 lines 0 comments Download

Messages

Total messages: 11 (1 generated)
cdotstout
5 years, 4 months ago (2015-07-31 17:28:12 UTC) #2
brettw
LGTM, although wait for a mojo reviewer before submitting (I don't understand the project enough ...
5 years, 4 months ago (2015-07-31 17:32:26 UTC) #3
cdotstout
https://codereview.chromium.org/1254963012/diff/1/mojo/public/dart/rules.gni File mojo/public/dart/rules.gni (right): https://codereview.chromium.org/1254963012/diff/1/mojo/public/dart/rules.gni#newcode130 mojo/public/dart/rules.gni:130: deps = [] On 2015/07/31 17:32:26, brettw wrote: > ...
5 years, 4 months ago (2015-07-31 17:37:15 UTC) #4
jamesr
https://codereview.chromium.org/1254963012/diff/20001/mojo/nacl/BUILD.gn File mojo/nacl/BUILD.gn (left): https://codereview.chromium.org/1254963012/diff/20001/mojo/nacl/BUILD.gn#oldcode58 mojo/nacl/BUILD.gn:58: "//shell:apptests", are the shell apptests built for nacl via ...
5 years, 4 months ago (2015-08-03 17:48:49 UTC) #5
cdotstout
https://codereview.chromium.org/1254963012/diff/20001/mojo/nacl/BUILD.gn File mojo/nacl/BUILD.gn (left): https://codereview.chromium.org/1254963012/diff/20001/mojo/nacl/BUILD.gn#oldcode58 mojo/nacl/BUILD.gn:58: "//shell:apptests", On 2015/08/03 17:48:49, jamesr wrote: > are the ...
5 years, 4 months ago (2015-08-03 18:14:58 UTC) #6
jamesr
On 2015/08/03 at 18:14:58, cstout wrote: > https://codereview.chromium.org/1254963012/diff/20001/mojo/nacl/BUILD.gn > File mojo/nacl/BUILD.gn (left): > > https://codereview.chromium.org/1254963012/diff/20001/mojo/nacl/BUILD.gn#oldcode58 ...
5 years, 4 months ago (2015-08-03 18:32:21 UTC) #7
jamesr
I think it'd be OK to disable the nacl shell_apptests target temporarily while we try ...
5 years, 4 months ago (2015-08-04 00:49:23 UTC) #8
jamesr
lgtm
5 years, 4 months ago (2015-08-04 22:13:26 UTC) #9
cdotstout
On 2015/08/04 00:49:23, jamesr wrote: > I think it'd be OK to disable the nacl ...
5 years, 4 months ago (2015-08-04 22:14:49 UTC) #10
cdotstout
5 years, 4 months ago (2015-08-04 22:15:26 UTC) #11
Message was sent while issue was closed.
Committed patchset #3 (id:40001) manually as
677be22e6e22bddecdbd1c1e5ac24dc0818c241f (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698