Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2697293002:
Enable asset tasks when using gradle to process resources. (Closed)
|
Created:
3 years, 10 months ago by sakal-chromium Modified:
3 years, 10 months ago CC:
chromium-reviews, mikecase+watch_chromium.org, jbudorick+watch_chromium.org, agrieve+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionEnable asset tasks when using gradle to process resources.
This is needed to correctly generate a project for AppRTCMobile.
BUG=620034, webrtc:6328
Review-Url: https://codereview.chromium.org/2697293002
Cr-Commit-Position: refs/heads/master@{#450985}
Committed: https://chromium.googlesource.com/chromium/src/+/41b7f2e4ce06ea76bb9dc23c752c6d1035ae6e27
Patch Set 1 #Patch Set 2 : Use process-gradle-resources flag. #Messages
Total messages: 15 (7 generated)
Description was changed from ========== Add --use-gradle-process-assets flag to generate_gradle.py. This is needed to correctly generate a project for AppRTCMobile. BUG=620034,webrtc:6328 ========== to ========== Add --use-gradle-process-assets flag to generate_gradle.py. This is needed to correctly generate a project for AppRTCMobile. BUG=620034,webrtc:6328 ==========
sakal@chromium.org changed reviewers: + agrieve@chromium.org, wnwen@chromium.org
PTAL
@agrieve - I think this can just go under the --use-gradle-process-resources flag, since to generate R.java we'd need the assets anyways. WDYT?
On 2017/02/16 14:26:21, Peter Wen wrote: > @agrieve - I think this can just go under the --use-gradle-process-resources > flag, since to generate R.java we'd need the assets anyways. WDYT? As far WebRTC build is concerned, sgtm.
On 2017/02/16 15:19:03, sakal-chromium wrote:
> On 2017/02/16 14:26:21, Peter Wen wrote:
> > @agrieve - I think this can just go under the --use-gradle-process-resources
> > flag, since to generate R.java we'd need the assets anyways. WDYT?
>
> As far WebRTC build is concerned, sgtm.
Note (afaict), this doesn't actually tell gradle where any assets are (doesn't
set assets.srcDirs), so this should essentially be a no-op.
Makes sense to put it under the same flag as resources.
If we do eventually tell it where the assets are, we'll also need to tell it
which ones to not compress via:
android {
aaptOptions {
noCompress 'foo', 'bar'
}
}
lgtm
On 2017/02/16 15:33:35, agrieve wrote:
> On 2017/02/16 15:19:03, sakal-chromium wrote:
> > On 2017/02/16 14:26:21, Peter Wen wrote:
> > > @agrieve - I think this can just go under the
--use-gradle-process-resources
> > > flag, since to generate R.java we'd need the assets anyways. WDYT?
> >
> > As far WebRTC build is concerned, sgtm.
>
> Note (afaict), this doesn't actually tell gradle where any assets are (doesn't
> set assets.srcDirs), so this should essentially be a no-op.
> Makes sense to put it under the same flag as resources.
>
> If we do eventually tell it where the assets are, we'll also need to tell it
> which ones to not compress via:
> android {
> aaptOptions {
> noCompress 'foo', 'bar'
> }
> }
>
> lgtm
lgtm
Sounds good, adding assets to the "todo" list. :)
Description was changed from ========== Add --use-gradle-process-assets flag to generate_gradle.py. This is needed to correctly generate a project for AppRTCMobile. BUG=620034,webrtc:6328 ========== to ========== Enable asset tasks when using gradle to process resources. This is needed to correctly generate a project for AppRTCMobile. BUG=620034,webrtc:6328 ==========
On 2017/02/16 15:34:54, Peter Wen wrote:
> On 2017/02/16 15:33:35, agrieve wrote:
> > On 2017/02/16 15:19:03, sakal-chromium wrote:
> > > On 2017/02/16 14:26:21, Peter Wen wrote:
> > > > @agrieve - I think this can just go under the
> --use-gradle-process-resources
> > > > flag, since to generate R.java we'd need the assets anyways. WDYT?
> > >
> > > As far WebRTC build is concerned, sgtm.
> >
> > Note (afaict), this doesn't actually tell gradle where any assets are
(doesn't
> > set assets.srcDirs), so this should essentially be a no-op.
> > Makes sense to put it under the same flag as resources.
> >
> > If we do eventually tell it where the assets are, we'll also need to tell it
> > which ones to not compress via:
> > android {
> > aaptOptions {
> > noCompress 'foo', 'bar'
> > }
> > }
> >
> > lgtm
>
> lgtm
>
> Sounds good, adding assets to the "todo" list. :)
Okay, I implemented this change.
The CQ bit was checked by sakal@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from wnwen@chromium.org, agrieve@chromium.org Link to the patchset: https://codereview.chromium.org/2697293002/#ps20001 (title: "Use process-gradle-resources flag.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1487260789961930,
"parent_rev": "014a8015a054e4f8b21f7b1a163297bd9f12c22a", "commit_rev":
"41b7f2e4ce06ea76bb9dc23c752c6d1035ae6e27"}
Message was sent while issue was closed.
Description was changed from ========== Enable asset tasks when using gradle to process resources. This is needed to correctly generate a project for AppRTCMobile. BUG=620034,webrtc:6328 ========== to ========== Enable asset tasks when using gradle to process resources. This is needed to correctly generate a project for AppRTCMobile. BUG=620034,webrtc:6328 Review-Url: https://codereview.chromium.org/2697293002 Cr-Commit-Position: refs/heads/master@{#450985} Committed: https://chromium.googlesource.com/chromium/src/+/41b7f2e4ce06ea76bb9dc23c752c... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/41b7f2e4ce06ea76bb9dc23c752c... |
