Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2131963002:
Early return if we find selected shipping option from last. (Closed)
|
Created:
4 years, 5 months ago by pals Modified:
4 years, 5 months ago Reviewers:
please use gerrit instead CC:
chromium-reviews, blink-reviews, haraken Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionEarly return if we find selected shipping option from last.
The spec requires using the last selected shipping option.
Instead of going through the whole list of shipping options
from front to back, iterate from back to front and early
return on the first selected shipping option.
BUG=587995
TEST=None (No behavior change)
Committed: https://crrev.com/8142cc7a7fe456a03d36343a4420d117b7207c1c
Cr-Commit-Position: refs/heads/master@{#404449}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Review comments fixed #Messages
Total messages: 17 (9 generated)
Description was changed from ========== Early return if we find selected shipping option from last. BUG=587995 ========== to ========== Early return if we find selected shipping option from last. BUG=587995 TEST=None (No behavior change) ==========
sanjoy.pal@samsung.com changed reviewers: + rouslan@chromium.org
PTAL.
The CQ bit was checked by sanjoy.pal@samsung.com 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...
Please explain in the patch description why early return is correct. For example, "The spec requires using the last selected shipping option. Instead of going through the whole list of shipping options from front to back, iterate from back to front and early-return on the first selected shipping option." https://codereview.chromium.org/2131963002/diff/1/third_party/WebKit/Source/m... File third_party/WebKit/Source/modules/payments/PaymentRequest.cpp (right): https://codereview.chromium.org/2131963002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:342: result = details.shippingOptions()[i].id(); return details.shippingOption()[i].id();
Description was changed from ========== Early return if we find selected shipping option from last. BUG=587995 TEST=None (No behavior change) ========== to ========== Early return if we find selected shipping option from last. The spec requires using the last selected shipping option. Instead of going through the whole list of shipping options from front to back, iterate from back to front and early return on the first selected shipping option. BUG=587995 TEST=None (No behavior change) ==========
Description was changed from ========== Early return if we find selected shipping option from last. The spec requires using the last selected shipping option. Instead of going through the whole list of shipping options from front to back, iterate from back to front and early return on the first selected shipping option. BUG=587995 TEST=None (No behavior change) ========== to ========== Early return if we find selected shipping option from last. The spec requires using the last selected shipping option. Instead of going through the whole list of shipping options from front to back, iterate from back to front and early return on the first selected shipping option. BUG=587995 TEST=None (No behavior change) ==========
Done. PTAL https://codereview.chromium.org/2131963002/diff/1/third_party/WebKit/Source/m... File third_party/WebKit/Source/modules/payments/PaymentRequest.cpp (right): https://codereview.chromium.org/2131963002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:342: result = details.shippingOptions()[i].id(); On 2016/07/08 13:53:47, Rouslan (ツ) wrote: > return details.shippingOption()[i].id(); Done.
lgtm
lgtm lgtm
The CQ bit was checked by sanjoy.pal@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Early return if we find selected shipping option from last. The spec requires using the last selected shipping option. Instead of going through the whole list of shipping options from front to back, iterate from back to front and early return on the first selected shipping option. BUG=587995 TEST=None (No behavior change) ========== to ========== Early return if we find selected shipping option from last. The spec requires using the last selected shipping option. Instead of going through the whole list of shipping options from front to back, iterate from back to front and early return on the first selected shipping option. BUG=587995 TEST=None (No behavior change) ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Early return if we find selected shipping option from last. The spec requires using the last selected shipping option. Instead of going through the whole list of shipping options from front to back, iterate from back to front and early return on the first selected shipping option. BUG=587995 TEST=None (No behavior change) ========== to ========== Early return if we find selected shipping option from last. The spec requires using the last selected shipping option. Instead of going through the whole list of shipping options from front to back, iterate from back to front and early return on the first selected shipping option. BUG=587995 TEST=None (No behavior change) Committed: https://crrev.com/8142cc7a7fe456a03d36343a4420d117b7207c1c Cr-Commit-Position: refs/heads/master@{#404449} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/8142cc7a7fe456a03d36343a4420d117b7207c1c Cr-Commit-Position: refs/heads/master@{#404449} |
