Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2756193003:
Use MDC CollectionCells bundle. (Closed)
DescriptionUse MDC CollectionCells bundle.
BUG=688096
R=sdefresne@chromium.org
Review-Url: https://codereview.chromium.org/2756193003
Cr-Commit-Position: refs/heads/master@{#459103}
Committed: https://chromium.googlesource.com/chromium/src/+/01de990dd9de1747f7c7c59a1e664e11933f66ad
Patch Set 1 #
Total comments: 2
Patch Set 2 : cl #
Total comments: 2
Messages
Total messages: 17 (7 generated)
Should I review this CL? You never sent an email.
On 2017/03/22 12:32:30, sdefresne wrote: > Should I review this CL? You never sent an email. Not yet, as the bundle doesn’t exist in MDC until today's release.
OK to review.
https://codereview.chromium.org/2756193003/diff/1/ios/third_party/material_co... File ios/third_party/material_components_ios/BUILD.gn (right): https://codereview.chromium.org/2756193003/diff/1/ios/third_party/material_co... ios/third_party/material_components_ios/BUILD.gn:271: bundle_data("material_collections_bundle") { I think "material_collections_bundle" and "material_collection_cells_bundle" should both have code for supporting multiple locales or just support "en". WDYT?
The CQ bit was checked by lpromero@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...
https://codereview.chromium.org/2756193003/diff/1/ios/third_party/material_co... File ios/third_party/material_components_ios/BUILD.gn (right): https://codereview.chromium.org/2756193003/diff/1/ios/third_party/material_co... ios/third_party/material_components_ios/BUILD.gn:271: bundle_data("material_collections_bundle") { On 2017/03/23 11:42:13, sdefresne wrote: > I think "material_collections_bundle" and "material_collection_cells_bundle" > should both have code for supporting multiple locales or just support "en". > WDYT? Makes sense. Done.
https://codereview.chromium.org/2756193003/diff/20001/ios/third_party/materia... File ios/third_party/material_components_ios/BUILD.gn (right): https://codereview.chromium.org/2756193003/diff/20001/ios/third_party/materia... ios/third_party/material_components_ios/BUILD.gn:273: foreach(locale, _locales) { Is it OK to use the same for loop for both bundle_data targets? Or do you prefer one foreach for each?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2756193003/diff/20001/ios/third_party/materia... File ios/third_party/material_components_ios/BUILD.gn (right): https://codereview.chromium.org/2756193003/diff/20001/ios/third_party/materia... ios/third_party/material_components_ios/BUILD.gn:273: foreach(locale, _locales) { On 2017/03/23 12:12:36, lpromero wrote: > Is it OK to use the same for loop for both bundle_data targets? Or do you prefer > one foreach for each? It's fine using the same loop (we can always split it if the list of supported locales is different).
The CQ bit was checked by lpromero@chromium.org
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": 1490284674663010,
"parent_rev": "2395c8174fa91a368e9184dae7ce2b50c45089a8", "commit_rev":
"01de990dd9de1747f7c7c59a1e664e11933f66ad"}
Message was sent while issue was closed.
Description was changed from ========== Use MDC CollectionCells bundle. BUG=688096 R=sdefresne@chromium.org ========== to ========== Use MDC CollectionCells bundle. BUG=688096 R=sdefresne@chromium.org Review-Url: https://codereview.chromium.org/2756193003 Cr-Commit-Position: refs/heads/master@{#459103} Committed: https://chromium.googlesource.com/chromium/src/+/01de990dd9de1747f7c7c59a1e66... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/01de990dd9de1747f7c7c59a1e66... |
