Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1533353002:
GN: asset_location -> android_assets() for cronet (Closed)
DescriptionGN: asset_location -> android_assets() for cronet
BUG=547162
Committed: https://crrev.com/540db42e43fc68920cc4898a93fb70a16e28a65d
Cr-Commit-Position: refs/heads/master@{#366508}
Patch Set 1 #
Total comments: 2
Patch Set 2 : list out files #
Total comments: 6
Patch Set 3 : testonly, sorted files, comment. #Messages
Total messages: 16 (5 generated)
agrieve@chromium.org changed reviewers: + mef@chromium.org
❇ ❈ ❊
https://codereview.chromium.org/1533353002/diff/1/components/cronet/android/B... File components/cronet/android/BUILD.gn (right): https://codereview.chromium.org/1533353002/diff/1/components/cronet/android/B... components/cronet/android/BUILD.gn:414: "test/assets/test", according to https://code.google.com/p/chromium/codesearch#chromium/src/build/config/andro... |sources| should list files, not directories.
https://codereview.chromium.org/1533353002/diff/1/components/cronet/android/B... File components/cronet/android/BUILD.gn (right): https://codereview.chromium.org/1533353002/diff/1/components/cronet/android/B... components/cronet/android/BUILD.gn:414: "test/assets/test", On 2015/12/21 16:50:14, mef wrote: > according to > https://code.google.com/p/chromium/codesearch#chromium/src/build/config/andro... > |sources| should list files, not directories. Right you are! Fixed.
https://codereview.chromium.org/1533353002/diff/20001/components/cronet/andro... File components/cronet/android/BUILD.gn (right): https://codereview.chromium.org/1533353002/diff/20001/components/cronet/andro... components/cronet/android/BUILD.gn:412: android_assets("cronet_test_apk_assets") { should have testonly = true https://codereview.chromium.org/1533353002/diff/20001/components/cronet/andro... components/cronet/android/BUILD.gn:423: "test/assets/test/multiredirect.html", could / should these be sorted alphabetically? https://codereview.chromium.org/1533353002/diff/20001/components/cronet/andro... components/cronet/android/BUILD.gn:441: renaming_destinations = rebase_path(renaming_sources, "test/assets") Could you clarify what how would this rename for example test/assets/test/cacheable.txt and test/assets/test/sdch/dict/LeQxM80O.mock-http-headers
https://codereview.chromium.org/1533353002/diff/20001/components/cronet/andro... File components/cronet/android/BUILD.gn (right): https://codereview.chromium.org/1533353002/diff/20001/components/cronet/andro... components/cronet/android/BUILD.gn:412: android_assets("cronet_test_apk_assets") { On 2015/12/21 18:21:31, mef wrote: > should have testonly = true Done. https://codereview.chromium.org/1533353002/diff/20001/components/cronet/andro... components/cronet/android/BUILD.gn:423: "test/assets/test/multiredirect.html", On 2015/12/21 18:21:31, mef wrote: > could / should these be sorted alphabetically? Done. https://codereview.chromium.org/1533353002/diff/20001/components/cronet/andro... components/cronet/android/BUILD.gn:441: renaming_destinations = rebase_path(renaming_sources, "test/assets") On 2015/12/21 18:21:32, mef wrote: > Could you clarify what how would this rename for example > > test/assets/test/cacheable.txt > and > test/assets/test/sdch/dict/LeQxM80O.mock-http-headers > > Done.
Thanks, LGTM!
The CQ bit was checked by agrieve@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1533353002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1533353002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by agrieve@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1533353002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1533353002/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== GN: asset_location -> android_assets() for cronet BUG=547162 ========== to ========== GN: asset_location -> android_assets() for cronet BUG=547162 Committed: https://crrev.com/540db42e43fc68920cc4898a93fb70a16e28a65d Cr-Commit-Position: refs/heads/master@{#366508} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/540db42e43fc68920cc4898a93fb70a16e28a65d Cr-Commit-Position: refs/heads/master@{#366508} |
