Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2103273002:
🎐 Default chromium_code = false for Android targets within third_party (Closed)
DescriptionDefault chromium_code = false for Android targets within third_party
This affects the following:
//third_party/jmake:jmake
//third_party/jsr-305:jsr_305_javalib
//third_party/gif_player:gif_player_java
//third_party/leakcanary:leakcanary_noop_java
BUG=621774
TBR=yusufo
Committed: https://crrev.com/d08fe288cd840f2505dbd083fc9d75db82582d05
Cr-Commit-Position: refs/heads/master@{#402926}
Patch Set 1 #
Total comments: 5
Messages
Total messages: 20 (7 generated)
The CQ bit was checked by agrieve@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
agrieve@chromium.org changed reviewers: + yfriedman@chromium.org, yusufo@chromium.org
https://codereview.chromium.org/2103273002/diff/1/build/config/android/intern... File build/config/android/internal_rules.gni (right): https://codereview.chromium.org/2103273002/diff/1/build/config/android/intern... build/config/android/internal_rules.gni:1996: sources = [ Is this a local variable? Prefix with "_" or are you intentionally overriding this value?
https://codereview.chromium.org/2103273002/diff/1/build/config/android/intern... File build/config/android/internal_rules.gni (right): https://codereview.chromium.org/2103273002/diff/1/build/config/android/intern... build/config/android/internal_rules.gni:1996: sources = [ On 2016/06/29 15:29:20, Yaron wrote: > Is this a local variable? Prefix with "_" or are you intentionally overriding > this value? This is currently the only way to do filtering in GN. set_sources_assignment_filter applies only to the variables named "sources" :P
https://codereview.chromium.org/2103273002/diff/1/build/config/android/intern... File build/config/android/internal_rules.gni (right): https://codereview.chromium.org/2103273002/diff/1/build/config/android/intern... build/config/android/internal_rules.gni:1996: sources = [ On 2016/06/29 17:47:42, agrieve wrote: > On 2016/06/29 15:29:20, Yaron wrote: > > Is this a local variable? Prefix with "_" or are you intentionally overriding > > this value? > > This is currently the only way to do filtering in GN. > set_sources_assignment_filter applies only to the variables named "sources" :P So there's no need to cache and restore it after? Sorry for the naive questions but my familiarity with this is pretty low
https://codereview.chromium.org/2103273002/diff/1/build/config/android/intern... File build/config/android/internal_rules.gni (right): https://codereview.chromium.org/2103273002/diff/1/build/config/android/intern... build/config/android/internal_rules.gni:1996: sources = [ On 2016/06/29 18:16:22, Yaron wrote: > On 2016/06/29 17:47:42, agrieve wrote: > > On 2016/06/29 15:29:20, Yaron wrote: > > > Is this a local variable? Prefix with "_" or are you intentionally > overriding > > > this value? > > > > This is currently the only way to do filtering in GN. > > set_sources_assignment_filter applies only to the variables named "sources" :P > > So there's no need to cache and restore it after? > Sorry for the naive questions but my familiarity with this is pretty low It's restored (back to []) on line 2004. GN also resets it on each scope enter / exit.
lgtm https://codereview.chromium.org/2103273002/diff/1/build/config/android/intern... File build/config/android/internal_rules.gni (right): https://codereview.chromium.org/2103273002/diff/1/build/config/android/intern... build/config/android/internal_rules.gni:1996: sources = [ On 2016/06/29 18:46:40, agrieve wrote: > On 2016/06/29 18:16:22, Yaron wrote: > > On 2016/06/29 17:47:42, agrieve wrote: > > > On 2016/06/29 15:29:20, Yaron wrote: > > > > Is this a local variable? Prefix with "_" or are you intentionally > > overriding > > > > this value? > > > > > > This is currently the only way to do filtering in GN. > > > set_sources_assignment_filter applies only to the variables named "sources" > :P > > > > So there's no need to cache and restore it after? > > Sorry for the naive questions but my familiarity with this is pretty low > > It's restored (back to []) on line 2004. GN also resets it on each scope enter / > exit. Ya, I getcha. I was wondering if sources could be non-empty before this
Description was changed from ========== Default chromium_code = false for Android targets within third_party This affects the following: //third_party/jmake:jmake //third_party/jsr-305:jsr_305_javalib //third_party/gif_player:gif_player_java //third_party/leakcanary:leakcanary_noop_java BUG=621774 ========== to ========== Default chromium_code = false for Android targets within third_party This affects the following: //third_party/jmake:jmake //third_party/jsr-305:jsr_305_javalib //third_party/gif_player:gif_player_java //third_party/leakcanary:leakcanary_noop_java BUG=621774 TBR=yusufo ==========
On 2016/06/29 18:48:49, Yaron wrote: > lgtm > > https://codereview.chromium.org/2103273002/diff/1/build/config/android/intern... > File build/config/android/internal_rules.gni (right): > > https://codereview.chromium.org/2103273002/diff/1/build/config/android/intern... > build/config/android/internal_rules.gni:1996: sources = [ > On 2016/06/29 18:46:40, agrieve wrote: > > On 2016/06/29 18:16:22, Yaron wrote: > > > On 2016/06/29 17:47:42, agrieve wrote: > > > > On 2016/06/29 15:29:20, Yaron wrote: > > > > > Is this a local variable? Prefix with "_" or are you intentionally > > > overriding > > > > > this value? > > > > > > > > This is currently the only way to do filtering in GN. > > > > set_sources_assignment_filter applies only to the variables named > "sources" > > :P > > > > > > So there's no need to cache and restore it after? > > > Sorry for the naive questions but my familiarity with this is pretty low > > > > It's restored (back to []) on line 2004. GN also resets it on each scope enter > / > > exit. > > Ya, I getcha. I was wondering if sources could be non-empty before this Yusuf is ooo. TBR'ing for custom_tabs_client part.
The CQ bit was checked by agrieve@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Default chromium_code = false for Android targets within third_party This affects the following: //third_party/jmake:jmake //third_party/jsr-305:jsr_305_javalib //third_party/gif_player:gif_player_java //third_party/leakcanary:leakcanary_noop_java BUG=621774 TBR=yusufo ========== to ========== Default chromium_code = false for Android targets within third_party This affects the following: //third_party/jmake:jmake //third_party/jsr-305:jsr_305_javalib //third_party/gif_player:gif_player_java //third_party/leakcanary:leakcanary_noop_java BUG=621774 TBR=yusufo ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
CQ bit was unchecked.
Message was sent while issue was closed.
Description was changed from ========== Default chromium_code = false for Android targets within third_party This affects the following: //third_party/jmake:jmake //third_party/jsr-305:jsr_305_javalib //third_party/gif_player:gif_player_java //third_party/leakcanary:leakcanary_noop_java BUG=621774 TBR=yusufo ========== to ========== Default chromium_code = false for Android targets within third_party This affects the following: //third_party/jmake:jmake //third_party/jsr-305:jsr_305_javalib //third_party/gif_player:gif_player_java //third_party/leakcanary:leakcanary_noop_java BUG=621774 TBR=yusufo Committed: https://crrev.com/d08fe288cd840f2505dbd083fc9d75db82582d05 Cr-Commit-Position: refs/heads/master@{#402926} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/d08fe288cd840f2505dbd083fc9d75db82582d05 Cr-Commit-Position: refs/heads/master@{#402926} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
