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

Issue 1107483002: [Chromoting] Update build-webapp to accept a filelist & update GN rules (Closed)

Created:
5 years, 8 months ago by garykac
Modified:
5 years, 8 months ago
Reviewers:
Jamie
CC:
chromium-reviews, chromoting-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[Chromoting] Update build-webapp to accept a filelist & update GN rules This cl: * Adds file list support to build-webapp.py * Update teh GN webapp build rules to build/pass a filelist * Enable the GN webapp build for Windows This change is motivated by enabling the remoting webapp build on Windows, which has a limit on the size of the command line. This limit was encountered with the Windows GN build. The corresponding GYP build files are not updated (even though GYP has support for building filelists via the <| syntax) because our files have GYP paths in them that get written into the file rather than being expanded. Fixing this is not worth the time since we're moving away from our GYP builds. BUG= Committed: https://crrev.com/c9f083641d5b6614195aa6561d82776dd668323d Cr-Commit-Position: refs/heads/master@{#326959}

Patch Set 1 #

Patch Set 2 : Add 's' to comment #

Total comments: 2

Patch Set 3 : Fix indent #

Unified diffs Side-by-side diffs Delta from patch set Stats (+34 lines, -14 lines) Patch
M remoting/BUILD.gn View 1 chunk +2 lines, -6 lines 0 comments Download
M remoting/webapp/build-webapp.py View 1 2 5 chunks +18 lines, -5 lines 0 comments Download
M remoting/webapp/build_template.gni View 1 2 chunks +14 lines, -3 lines 0 comments Download

Messages

Total messages: 10 (3 generated)
garykac
5 years, 8 months ago (2015-04-23 14:44:27 UTC) #2
garykac
On 2015/04/23 14:44:27, garykac wrote: ping!
5 years, 8 months ago (2015-04-24 23:46:13 UTC) #3
Jamie
lgtm https://codereview.chromium.org/1107483002/diff/20001/remoting/webapp/build-webapp.py File remoting/webapp/build-webapp.py (right): https://codereview.chromium.org/1107483002/diff/20001/remoting/webapp/build-webapp.py#newcode136 remoting/webapp/build-webapp.py:136: line, identifying the resources to include in this ...
5 years, 8 months ago (2015-04-24 23:51:49 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1107483002/40001
5 years, 8 months ago (2015-04-25 00:23:56 UTC) #7
commit-bot: I haz the power
Committed patchset #3 (id:40001)
5 years, 8 months ago (2015-04-25 04:53:48 UTC) #8
commit-bot: I haz the power
Patchset 3 (id:??) landed as https://crrev.com/c9f083641d5b6614195aa6561d82776dd668323d Cr-Commit-Position: refs/heads/master@{#326959}
5 years, 8 months ago (2015-04-25 04:55:56 UTC) #9
garykac
5 years, 8 months ago (2015-04-27 15:36:16 UTC) #10
Message was sent while issue was closed.
https://codereview.chromium.org/1107483002/diff/20001/remoting/webapp/build-w...
File remoting/webapp/build-webapp.py (right):

https://codereview.chromium.org/1107483002/diff/20001/remoting/webapp/build-w...
remoting/webapp/build-webapp.py:136: line, identifying the resources to include
in this webapp. This
On 2015/04/24 23:51:49, Jamie wrote:
> Nit: Indentation.

Done.

Powered by Google App Engine
This is Rietveld 408576698