Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2614193002:
[Payments] List incomplete addresses in the billing address dropdown (Closed)
|
Created:
3 years, 11 months ago by gogerald1 Modified:
3 years, 11 months ago CC:
chromium-reviews, rouslan+payments_chromium.org, sebsg+paymentswatch_chromium.org, gogerald+paymentswatch_chromium.org, agrieve+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionList incomplete addresses in the billing address dropdown
The billing addresses in the dropdown contain edit required information
and are sorted by completeness.
Select incomplete address brings up address editor.
Select previously selected address or null if user cancels from address editor.
Select and add the newly added or completed address to the top of the dropdown.
BUG=675686
Review-Url: https://codereview.chromium.org/2614193002
Cr-Commit-Position: refs/heads/master@{#442657}
Committed: https://chromium.googlesource.com/chromium/src/+/08b780f0ee9b1c60724ad8c8024bd84cc8a6d928
Patch Set 1 #
Total comments: 12
Patch Set 2 : address comments #
Total comments: 2
Patch Set 3 : rebase #
Total comments: 9
Patch Set 4 : fix nits #Messages
Total messages: 54 (38 generated)
Description was changed from ========== order list incomplete addressess in billing address dropdown with sorting capability worked well draft worked draft draft BUG= ========== to ========== List incomplete addresses in the billing address dropdown BUG=675686 ==========
Description was changed from ========== List incomplete addresses in the billing address dropdown BUG=675686 ========== to ========== List incomplete addresses in the billing address dropdown The billing addresses in the dropdown are sorted by completeness. Select incomplete address brings up address editor. Select previously selected address or null if user cancels edition. Select and add the newly added or edited address to the top of the dropdown if user finishes edition. BUG=675686 ==========
Description was changed from ========== List incomplete addresses in the billing address dropdown The billing addresses in the dropdown are sorted by completeness. Select incomplete address brings up address editor. Select previously selected address or null if user cancels edition. Select and add the newly added or edited address to the top of the dropdown if user finishes edition. BUG=675686 ========== to ========== List incomplete addresses in the billing address dropdown The billing addresses in the dropdown are sorted by completeness. Select incomplete address brings up address editor. Select previously selected address or null if user cancels edition. Select and add the newly added or edited address to the top of the dropdown if user finishes edition. BUG=675686 ==========
Description was changed from ========== List incomplete addresses in the billing address dropdown The billing addresses in the dropdown are sorted by completeness. Select incomplete address brings up address editor. Select previously selected address or null if user cancels edition. Select and add the newly added or edited address to the top of the dropdown if user finishes edition. BUG=675686 ========== to ========== List incomplete addresses in the billing address dropdown The billing addresses in the dropdown are sorted by completeness. Select incomplete address brings up address editor. Select previously selected address or null if user cancels edition. Select and add the newly added or edited address to the top of the dropdown if user completes edition. BUG=675686 ==========
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Patchset #1 (id:40001) has been deleted
Description was changed from ========== List incomplete addresses in the billing address dropdown The billing addresses in the dropdown are sorted by completeness. Select incomplete address brings up address editor. Select previously selected address or null if user cancels edition. Select and add the newly added or edited address to the top of the dropdown if user completes edition. BUG=675686 ========== to ========== List incomplete addresses in the billing address dropdown The billing addresses in the dropdown contain edit required information and are sorted by completeness. Select incomplete address brings up address editor. Select previously selected address or null if user cancels edition. Select and add the newly added or edited address to the top of the dropdown if user completes edition. BUG=675686 ==========
Description was changed from ========== List incomplete addresses in the billing address dropdown The billing addresses in the dropdown contain edit required information and are sorted by completeness. Select incomplete address brings up address editor. Select previously selected address or null if user cancels edition. Select and add the newly added or edited address to the top of the dropdown if user completes edition. BUG=675686 ========== to ========== List incomplete addresses in the billing address dropdown The billing addresses in the dropdown contain edit required information and are sorted by completeness. Select incomplete address brings up address editor. Select previously selected address or null if user cancels from address editor. Select and add the newly added or completed address to the top of the dropdown. BUG=675686 ==========
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...
The CQ bit was unchecked by commit-bot@chromium.org
No L-G-T-M from a valid reviewer yet. CQ run can only be started by full committers or once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer, _not_ a full super star committer. See http://www.chromium.org/getting-involved/become-a-committer Note that this has nothing to do with OWNERS files.
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...
gogerald@chromium.org changed reviewers: + rouslan@chromium.org, tedchoc@chromium.org
Hi tedchoc@ and rouslan@, ptal,
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2614193002/diff/60001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java (right): https://codereview.chromium.org/2614193002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java:175: * Gets the edit message and title resource Ids for the completion status. +1 indent for lines 175-181. https://codereview.chromium.org/2614193002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java:178: * @return Pair<Integer, Integer> The resource Ids. The first integer is the edit message Don't put the type in @return JavaDoc, please. https://codereview.chromium.org/2614193002/diff/60001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java (right): https://codereview.chromium.org/2614193002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java:103: private final Map<String, Integer> mInCompleteProfilesForBillingAddress; "incomplete" is one word, so the "C" should not be capitalized in camel-casing: mIncompleteProfilesForBillingAddress Otherwise, a reader might be confused by "in complete profiles" phrase. https://codereview.chromium.org/2614193002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java:188: boolean isAComplete = You seem to have defined isAComplete based on the completion status of "b" and vice versa. https://codereview.chromium.org/2614193002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java:600: Object sizeSpanner = new AbsoluteSizeSpan(14, true); Is there a constant for 14 anywhere? https://codereview.chromium.org/2614193002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java:696: private AutofillProfile findTargetProfile(List<AutofillProfile> profiles, String guid) { static
https://codereview.chromium.org/2614193002/diff/60001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java (right): https://codereview.chromium.org/2614193002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java:175: * Gets the edit message and title resource Ids for the completion status. On 2017/01/09 18:36:30, rouslan wrote: > +1 indent for lines 175-181. Done. https://codereview.chromium.org/2614193002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java:178: * @return Pair<Integer, Integer> The resource Ids. The first integer is the edit message On 2017/01/09 18:36:30, rouslan wrote: > Don't put the type in @return JavaDoc, please. Done. https://codereview.chromium.org/2614193002/diff/60001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java (right): https://codereview.chromium.org/2614193002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java:103: private final Map<String, Integer> mInCompleteProfilesForBillingAddress; On 2017/01/09 18:36:30, rouslan wrote: > "incomplete" is one word, so the "C" should not be capitalized in camel-casing: > > mIncompleteProfilesForBillingAddress > > Otherwise, a reader might be confused by "in complete profiles" phrase. Done. https://codereview.chromium.org/2614193002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java:188: boolean isAComplete = On 2017/01/09 18:36:30, rouslan wrote: > You seem to have defined isAComplete based on the completion status of "b" and > vice versa. Done. https://codereview.chromium.org/2614193002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java:600: Object sizeSpanner = new AbsoluteSizeSpan(14, true); On 2017/01/09 18:36:30, rouslan wrote: > Is there a constant for 14 anywhere? This size is the text size in the dropdown which is defined by the Android default spinner item style. Add a comment to explain it. https://codereview.chromium.org/2614193002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java:696: private AutofillProfile findTargetProfile(List<AutofillProfile> profiles, String guid) { On 2017/01/09 18:36:30, rouslan wrote: > static Done.
Need a rebase in CardEditor.java.
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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
Patchset #3 (id:100001) has been deleted
https://codereview.chromium.org/2614193002/diff/80001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java (right): https://codereview.chromium.org/2614193002/diff/80001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java:621: mBillingAddressField.setRequiredErrorMessage( By the way, the billing address dropdown now has multiple invalid states. Before this patch, only key "" was invalid. The "field required" property handled that error state. Now every key that matches one of the mIncompleteProfilesForBillingAddress keys is also invalid. You can reproduce the following crash with this patch as-is: 1) Setup Chrome with a local credit card, whose billing address is missing the recipient name. 2) Open https://rsolomakhin.github.io/pr/cc/. 3) Click "Buy" on page. 4) Click "SELECT" in the "Payment" section. 5) Select the credit card from step (1). 6) Click "DONE" button in the card editor. Observed: Crash. Expected: The selected billing address should be marked invalid.
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...
https://codereview.chromium.org/2614193002/diff/80001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java (right): https://codereview.chromium.org/2614193002/diff/80001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java:621: mBillingAddressField.setRequiredErrorMessage( On 2017/01/09 20:29:19, rouslan wrote: > By the way, the billing address dropdown now has multiple invalid states. Before > this patch, only key "" was invalid. The "field required" property handled that > error state. Now every key that matches one of the > mIncompleteProfilesForBillingAddress keys is also invalid. > > You can reproduce the following crash with this patch as-is: > > 1) Setup Chrome with a local credit card, whose billing address is missing the > recipient name. > 2) Open https://rsolomakhin.github.io/pr/cc/. > 3) Click "Buy" on page. > 4) Click "SELECT" in the "Payment" section. > 5) Select the credit card from step (1). > 6) Click "DONE" button in the card editor. > > Observed: Crash. > Expected: The selected billing address should be marked invalid. Thanks for catching this. Here we might have few options 1, show 'SELECT' hint in cardEditor for billing address field, 2, show selected billing address with edit required information and bring up address editor when clicking 'Done' in cardEditor, 3, show selected billing address with edit required information and introduce billing address validator to indicate error when clicking 'Done' in cardEditor. Do not have strong preference to any of them, but option 1 looks clear and simple. It also makes sense since we have set the billing address to null for instrument to show "Billing address required".
Option 1 is the best. On Jan 10, 2017 9:11 AM, <gogerald@chromium.org> wrote: > > https://codereview.chromium.org/2614193002/diff/80001/ > chrome/android/java/src/org/chromium/chrome/browser/ > payments/CardEditor.java > File > chrome/android/java/src/org/chromium/chrome/browser/ > payments/CardEditor.java > (right): > > https://codereview.chromium.org/2614193002/diff/80001/ > chrome/android/java/src/org/chromium/chrome/browser/ > payments/CardEditor.java#newcode621 > chrome/android/java/src/org/chromium/chrome/browser/ > payments/CardEditor.java:621: > mBillingAddressField.setRequiredErrorMessage( > On 2017/01/09 20:29:19, rouslan wrote: > > By the way, the billing address dropdown now has multiple invalid > states. Before > > this patch, only key "" was invalid. The "field required" property > handled that > > error state. Now every key that matches one of the > > mIncompleteProfilesForBillingAddress keys is also invalid. > > > > You can reproduce the following crash with this patch as-is: > > > > 1) Setup Chrome with a local credit card, whose billing address is > missing the > > recipient name. > > 2) Open https://rsolomakhin.github.io/pr/cc/. > > 3) Click "Buy" on page. > > 4) Click "SELECT" in the "Payment" section. > > 5) Select the credit card from step (1). > > 6) Click "DONE" button in the card editor. > > > > Observed: Crash. > > Expected: The selected billing address should be marked invalid. > > Thanks for catching this. > > Here we might have few options > > 1, show 'SELECT' hint in cardEditor for billing address field, > > 2, show selected billing address with edit required information and > bring up address editor when clicking 'Done' in cardEditor, > > 3, show selected billing address with edit required information and > introduce billing address validator to indicate error when clicking > 'Done' in cardEditor. > > Do not have strong preference to any of them, but option 1 looks clear > and simple. It also makes sense since we have set the billing address to > null for instrument to show "Billing address required". > > https://codereview.chromium.org/2614193002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Agreed, I've implemented this option in the rebase patch and updated the tests to cover this logic. please take another look.
LGTM, but please wait for Ted to review the spannable code.
lgtm...nits and small suggestions that I'll defer to you about. https://codereview.chromium.org/2614193002/diff/120001/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java (right): https://codereview.chromium.org/2614193002/diff/120001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java:181: public static Pair<Integer, Integer> getEditMessageAndTitleResIds( Any thought on having two functions, getEditMessageResId and getEditTitleResId? The pair makes it less clear to read in other places IMO. https://codereview.chromium.org/2614193002/diff/120001/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java (right): https://codereview.chromium.org/2614193002/diff/120001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java:199: return (isBComplete ? 1 : 0) - (isAComplete ? 1 : 0); One thing to consider would be to add compareBoolean to ApiCompabilityUtils (to match compareLong). Then just copy the logic from Android: return lhs == rhs ? 0 : lhs ? 1 : -1; "slightly" more consistent with our codebase. https://codereview.chromium.org/2614193002/diff/120001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java:640: builder.setSpan(sizeSpanner, startIndex, builder.length(), 0); I assume this follows the pattern for how we build these strings in general. My concern would be around RTL or other languages where this potentially looks out of place. If so, my only suggestion here is around readability nits ----------- builder.append(mContext.getString(R.string.autofill_address_summary_separator)); int startIndex = builder.length(); int editMessageResId = mIncompleteProfilesForBillingAddress.get(profile.getGUID()).intValue(); builder.append(mContext.getString(editMessageResId)); int endIndex = builder.length(); Object foregroundSpan = new ForegroundColorSpan(ApiCompatibilityUtils.getColor( mContext.getResources(), R.color.google_blue_700)); builder.setSpan(foregroundSpan, startIndex, endIndex, 0); Object sizeSpan = new AbsoluteSizeSpan(14, true); builder.setSpan(sizeSpan, startIndex, endIndex, 0); ------------ Makes each component a bit more digestible IMO. https://codereview.chromium.org/2614193002/diff/120001/chrome/android/javates... File chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestBillingAddressTest.java (right): https://codereview.chromium.org/2614193002/diff/120001/chrome/android/javates... chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestBillingAddressTest.java:312: .equals("Select")); indented too much here (should be 8 from the start of the line)...same below
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...
Thanks, https://codereview.chromium.org/2614193002/diff/120001/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java (right): https://codereview.chromium.org/2614193002/diff/120001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java:181: public static Pair<Integer, Integer> getEditMessageAndTitleResIds( On 2017/01/10 17:07:19, Ted C wrote: > Any thought on having two functions, getEditMessageResId and getEditTitleResId? > The pair makes it less clear to read in other places IMO. Agreed that two functions looks a little clearer from outside, but I thought implement in one function looks slightly easier to maintain since message and title are strongly related to each other (Paired). In addition, this interface is only be used by payment instead of the other components, so it might not be a "big" concern. Leave it like this for now. https://codereview.chromium.org/2614193002/diff/120001/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java (right): https://codereview.chromium.org/2614193002/diff/120001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java:199: return (isBComplete ? 1 : 0) - (isAComplete ? 1 : 0); On 2017/01/10 17:07:19, Ted C wrote: > One thing to consider would be to add compareBoolean to ApiCompabilityUtils (to > match compareLong). > > Then just copy the logic from Android: > > return lhs == rhs ? 0 : lhs ? 1 : -1; > > "slightly" more consistent with our codebase. Seems no "big" benefit especially if we create Boolean instance for it, leave it for now, https://codereview.chromium.org/2614193002/diff/120001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java:640: builder.setSpan(sizeSpanner, startIndex, builder.length(), 0); On 2017/01/10 17:07:19, Ted C wrote: > I assume this follows the pattern for how we build these strings in general. My > concern would be around RTL or other languages where this potentially looks out > of place. > > If so, my only suggestion here is around readability nits > > ----------- > > builder.append(mContext.getString(R.string.autofill_address_summary_separator)); > > int startIndex = builder.length(); > int editMessageResId = > mIncompleteProfilesForBillingAddress.get(profile.getGUID()).intValue(); > builder.append(mContext.getString(editMessageResId)); > int endIndex = builder.length(); > > Object foregroundSpan = new ForegroundColorSpan(ApiCompatibilityUtils.getColor( > mContext.getResources(), R.color.google_blue_700)); > builder.setSpan(foregroundSpan, startIndex, endIndex, 0); > > Object sizeSpan = new AbsoluteSizeSpan(14, true); > builder.setSpan(sizeSpan, startIndex, endIndex, 0); > > ------------ > > Makes each component a bit more digestible IMO. Done. https://codereview.chromium.org/2614193002/diff/120001/chrome/android/javates... File chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestBillingAddressTest.java (right): https://codereview.chromium.org/2614193002/diff/120001/chrome/android/javates... chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestBillingAddressTest.java:312: .equals("Select")); On 2017/01/10 17:07:19, Ted C wrote: > indented too much here (should be 8 from the start of the line)...same below Done.
https://codereview.chromium.org/2614193002/diff/120001/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java (right): https://codereview.chromium.org/2614193002/diff/120001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java:199: return (isBComplete ? 1 : 0) - (isAComplete ? 1 : 0); On 2017/01/10 18:27:02, gogerald1 wrote: > On 2017/01/10 17:07:19, Ted C wrote: > > One thing to consider would be to add compareBoolean to ApiCompabilityUtils > (to > > match compareLong). > > > > Then just copy the logic from Android: > > > > return lhs == rhs ? 0 : lhs ? 1 : -1; > > > > "slightly" more consistent with our codebase. > > Seems no "big" benefit especially if we create Boolean instance for it, leave it > for now, FYI, there is no capital B boolean created for my proposed solution. /** * @see Boolean#compare(boolean, boolean) */ public static int compareBoolean(boolean lhs, boolean rhs) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) { return Boolean.compare(lhs, rhs); } else { return lhs == rhs ? 0 : lhs ? 1 : -1; } }
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Patchset #5 (id:160001) has been deleted
The CQ bit was checked by gogerald@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from tedchoc@chromium.org, rouslan@chromium.org Link to the patchset: https://codereview.chromium.org/2614193002/#ps140001 (title: "fix nits")
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": 140001, "attempt_start_ts": 1484076225113580,
"parent_rev": "849c6f79f019664a0cf223e9a03157c5c4bf8e80", "commit_rev":
"08b780f0ee9b1c60724ad8c8024bd84cc8a6d928"}
Message was sent while issue was closed.
Description was changed from ========== List incomplete addresses in the billing address dropdown The billing addresses in the dropdown contain edit required information and are sorted by completeness. Select incomplete address brings up address editor. Select previously selected address or null if user cancels from address editor. Select and add the newly added or completed address to the top of the dropdown. BUG=675686 ========== to ========== List incomplete addresses in the billing address dropdown The billing addresses in the dropdown contain edit required information and are sorted by completeness. Select incomplete address brings up address editor. Select previously selected address or null if user cancels from address editor. Select and add the newly added or completed address to the top of the dropdown. BUG=675686 Review-Url: https://codereview.chromium.org/2614193002 Cr-Commit-Position: refs/heads/master@{#442657} Committed: https://chromium.googlesource.com/chromium/src/+/08b780f0ee9b1c60724ad8c8024b... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:140001) as https://chromium.googlesource.com/chromium/src/+/08b780f0ee9b1c60724ad8c8024b... |
