|
|
Chromium Code Reviews|
Created:
3 years, 5 months ago by Moe Modified:
3 years, 5 months ago Reviewers:
macourteau CC:
chromium-reviews, gogerald+paymentswatch_chromium.org, ios-reviews+chrome_chromium.org, ios-reviews_chromium.org, mahmadi+paymentsioswatch_chromium.org, mahmadi+paymentswatch_chromium.org, marq+watch_chromium.org, noyau+watch_chromium.org, rouslan+payments_chromium.org, sebsg+paymentswatch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Description[Payment Request] Populates country field if country code/name is valid.
Compares autofill profile's country against available country codes and
country names. This is similar to what is being done for regions and should
make selecting the value for the country field more resilient to changes in
autofill's backend.
BUG=602666
Review-Url: https://codereview.chromium.org/2964333002
Cr-Commit-Position: refs/heads/master@{#484125}
Committed: https://chromium.googlesource.com/chromium/src/+/ad87709594ecb626423d4281db46a41d60735f8a
Patch Set 1 #
Total comments: 4
Patch Set 2 : Addressed comments #Messages
Total messages: 16 (10 generated)
The CQ bit was checked by mahmadi@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.
Description was changed from ========== [Payment Request] Populates country field if country code/name is valid. Compares autofill profile's country against avaialable country codes and cuntry names. BUG=602666 ========== to ========== [Payment Request] Populates country field if country code/name is valid. Compares autofill profile's country against available country codes and country names. This is similar to what is being done for regions and should make selecting the value for the country field more resilient to changes in autofill's backend. BUG=602666 ==========
mahmadi@chromium.org changed reviewers: + macourteau@chromium.org
Hi MAC, Please take a look whenever you have time.
lgtm % comments https://codereview.chromium.org/2964333002/diff/1/ios/chrome/browser/ui/payme... File ios/chrome/browser/ui/payments/address_edit_mediator.mm (right): https://codereview.chromium.org/2964333002/diff/1/ios/chrome/browser/ui/payme... ios/chrome/browser/ui/payments/address_edit_mediator.mm:192: // If an address is being edited and it has is a valid country code or a valid s/has // https://codereview.chromium.org/2964333002/diff/1/ios/chrome/browser/ui/payme... ios/chrome/browser/ui/payments/address_edit_mediator.mm:203: DCHECK(1 == [[countries allKeysForObject:country] count]); DCHECK -> DCHECK_EQ
Addressed comments https://codereview.chromium.org/2964333002/diff/1/ios/chrome/browser/ui/payme... File ios/chrome/browser/ui/payments/address_edit_mediator.mm (right): https://codereview.chromium.org/2964333002/diff/1/ios/chrome/browser/ui/payme... ios/chrome/browser/ui/payments/address_edit_mediator.mm:192: // If an address is being edited and it has is a valid country code or a valid On 2017/07/04 at 13:53:45, macourteau wrote: > s/has // Done. https://codereview.chromium.org/2964333002/diff/1/ios/chrome/browser/ui/payme... ios/chrome/browser/ui/payments/address_edit_mediator.mm:203: DCHECK(1 == [[countries allKeysForObject:country] count]); On 2017/07/04 at 13:53:45, macourteau wrote: > DCHECK -> DCHECK_EQ Done.
On 2017/07/04 at 15:47:06, Moe wrote: > Addressed comments > > https://codereview.chromium.org/2964333002/diff/1/ios/chrome/browser/ui/payme... > File ios/chrome/browser/ui/payments/address_edit_mediator.mm (right): > > https://codereview.chromium.org/2964333002/diff/1/ios/chrome/browser/ui/payme... > ios/chrome/browser/ui/payments/address_edit_mediator.mm:192: // If an address is being edited and it has is a valid country code or a valid > On 2017/07/04 at 13:53:45, macourteau wrote: > > s/has // > > Done. > > https://codereview.chromium.org/2964333002/diff/1/ios/chrome/browser/ui/payme... > ios/chrome/browser/ui/payments/address_edit_mediator.mm:203: DCHECK(1 == [[countries allKeysForObject:country] count]); > On 2017/07/04 at 13:53:45, macourteau wrote: > > DCHECK -> DCHECK_EQ > > Done. Thank you.
The CQ bit was checked by mahmadi@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from macourteau@chromium.org Link to the patchset: https://codereview.chromium.org/2964333002/#ps20001 (title: "Addressed comments")
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": 1499183236326640,
"parent_rev": "7a0173a58d82f11c7ea4463329a5b6dca15b9010", "commit_rev":
"ad87709594ecb626423d4281db46a41d60735f8a"}
Message was sent while issue was closed.
Description was changed from ========== [Payment Request] Populates country field if country code/name is valid. Compares autofill profile's country against available country codes and country names. This is similar to what is being done for regions and should make selecting the value for the country field more resilient to changes in autofill's backend. BUG=602666 ========== to ========== [Payment Request] Populates country field if country code/name is valid. Compares autofill profile's country against available country codes and country names. This is similar to what is being done for regions and should make selecting the value for the country field more resilient to changes in autofill's backend. BUG=602666 Review-Url: https://codereview.chromium.org/2964333002 Cr-Commit-Position: refs/heads/master@{#484125} Committed: https://chromium.googlesource.com/chromium/src/+/ad87709594ecb626423d4281db46... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/ad87709594ecb626423d4281db46... |
