Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2797223002:
[Payments] Use default country code if country code is not available in profile (Closed)
|
Created:
3 years, 8 months ago by gogerald1 Modified:
3 years, 8 months ago Reviewers:
please use gerrit instead CC:
chromium-reviews, mahmadi+paymentswatch_chromium.org, rouslan+payments_chromium.org, agrieve+watch_chromium.org, sebsg+paymentswatch_chromium.org, gogerald+paymentswatch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionUse default country code if country code is not available in profile
BUG=708686
Review-Url: https://codereview.chromium.org/2797223002
Cr-Commit-Position: refs/heads/master@{#462164}
Committed: https://chromium.googlesource.com/chromium/src/+/52ec5bf083e5cbd9f4e58672bde3624d9446bb8b
Patch Set 1 #
Total comments: 4
Patch Set 2 : use country field value #Patch Set 3 : fix compile #Messages
Total messages: 21 (15 generated)
Description was changed from ========== fix country code BUG= ========== to ========== Use default country code if country code is not available in profile BUG=708686 ==========
gogerald@chromium.org changed reviewers: + rouslan@chromium.org
The CQ bit was checked by gogerald@chromium.org to run a CQ dry run
Hi rouslan@, PTAL,
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/2797223002/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/payments/AddressEditor.java (right): https://codereview.chromium.org/2797223002/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/payments/AddressEditor.java:114: mCountryField.setValue(AutofillAddress.getCountryCode(profile)); profile.setCountryCode(AutofillAddress.getCountryCode(profile)); mCountryField.setValue(profile.getCountryCode()); https://codereview.chromium.org/2797223002/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/payments/AddressEditor.java:156: editor, AutofillAddress.getCountryCode(profile), profile.getLanguageCode()); AutofillAddress.getCountryCode() is expensive, because it uses regex. Let's call it only once on line 113. and put its output in |profile|. Here on line 155, let's keep using profile.getcountryCode().
The CQ bit was checked by gogerald@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...
Used mCountryField.getValue(), WDYT? https://codereview.chromium.org/2797223002/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/payments/AddressEditor.java (right): https://codereview.chromium.org/2797223002/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/payments/AddressEditor.java:114: mCountryField.setValue(AutofillAddress.getCountryCode(profile)); On 2017/04/05 17:33:24, ಠ_ಠ wrote: > profile.setCountryCode(AutofillAddress.getCountryCode(profile)); > mCountryField.setValue(profile.getCountryCode()); Use has not committed the changes, so it might not good to set it in profile for now. https://codereview.chromium.org/2797223002/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/payments/AddressEditor.java:156: editor, AutofillAddress.getCountryCode(profile), profile.getLanguageCode()); On 2017/04/05 17:33:24, ಠ_ಠ wrote: > AutofillAddress.getCountryCode() is expensive, because it uses regex. Let's call > it only once on line 113. and put its output in |profile|. Here on line 155, > let's keep using profile.getcountryCode(). mCountryField.getValue() looks more natural here.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
The CQ bit was checked by gogerald@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...
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by gogerald@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": 40001, "attempt_start_ts": 1491418211647540,
"parent_rev": "f540a38ed8f32aaa1b164faa5c17ca6349d20979", "commit_rev":
"52ec5bf083e5cbd9f4e58672bde3624d9446bb8b"}
Message was sent while issue was closed.
Description was changed from ========== Use default country code if country code is not available in profile BUG=708686 ========== to ========== Use default country code if country code is not available in profile BUG=708686 Review-Url: https://codereview.chromium.org/2797223002 Cr-Commit-Position: refs/heads/master@{#462164} Committed: https://chromium.googlesource.com/chromium/src/+/52ec5bf083e5cbd9f4e58672bde3... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/52ec5bf083e5cbd9f4e58672bde3... |
