Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2899083005:
[Payments] Fix metric log. (Closed)
|
Created:
3 years, 7 months ago by sebsg Modified:
3 years, 7 months ago Reviewers:
gogerald1 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. |
Description[Payments] Fix metric log.
BUG=725960
Review-Url: https://codereview.chromium.org/2899083005
Cr-Commit-Position: refs/heads/master@{#474387}
Committed: https://chromium.googlesource.com/chromium/src/+/9ae43cad500350d2637d42a972f11d8b7489879a
Patch Set 1 #Patch Set 2 : Fixed test #
Total comments: 6
Patch Set 3 : Updated function name and comment #
Messages
Total messages: 21 (13 generated)
sebsg@chromium.org changed reviewers: + gogerald@chromium.org
PTAL?
Description was changed from ========== [Payments] Fix metric log. BUG= ========== to ========== [Payments] Fix metric log. BUG=725960 ==========
lgtm
The CQ bit was checked by sebsg@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Hey Ganggui, I was going too fast, here is the test fix
The CQ bit was checked by sebsg@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.
still lgtm with comments https://codereview.chromium.org/2899083005/diff/20001/chrome/android/javatest... File chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestCanMakePaymentMetricsTest.java (right): https://codereview.chromium.org/2899083005/diff/20001/chrome/android/javatest... chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestCanMakePaymentMetricsTest.java:63: public void testCannotMakePayment_UserDismiss() it looks slightly better to call it "*UserAbort" to be consistent with the comemnts and the variable names in code. https://codereview.chromium.org/2899083005/diff/20001/chrome/android/javatest... chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestCanMakePaymentMetricsTest.java:154: * calling it, receiving yeas as a response, showing the Payment Request and the user aborts the *yes*? *merchant aborts*? https://codereview.chromium.org/2899083005/diff/20001/chrome/android/javatest... chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestCanMakePaymentMetricsTest.java:241: public void testNoQuery_UserDismiss() *UserAbort?
Patchset #3 (id:40001) has been deleted
Patchset #3 (id:60001) has been deleted
Thanks, sending to CQ https://codereview.chromium.org/2899083005/diff/20001/chrome/android/javatest... File chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestCanMakePaymentMetricsTest.java (right): https://codereview.chromium.org/2899083005/diff/20001/chrome/android/javatest... chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestCanMakePaymentMetricsTest.java:63: public void testCannotMakePayment_UserDismiss() On 2017/05/24 17:52:14, gogerald1 wrote: > it looks slightly better to call it "*UserAbort" to be consistent with the > comemnts and the variable names in code. Done. https://codereview.chromium.org/2899083005/diff/20001/chrome/android/javatest... chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestCanMakePaymentMetricsTest.java:154: * calling it, receiving yeas as a response, showing the Payment Request and the user aborts the On 2017/05/24 17:52:14, gogerald1 wrote: > *yes*? *merchant aborts*? Done. https://codereview.chromium.org/2899083005/diff/20001/chrome/android/javatest... chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestCanMakePaymentMetricsTest.java:241: public void testNoQuery_UserDismiss() On 2017/05/24 17:52:14, gogerald1 wrote: > *UserAbort? Done.
The CQ bit was checked by sebsg@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from gogerald@chromium.org Link to the patchset: https://codereview.chromium.org/2899083005/#ps80001 (title: "Updated function name and comment")
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": 80001, "attempt_start_ts": 1495648623094830,
"parent_rev": "335471d2d07caccbd4a89d67d86f777cf85941da", "commit_rev":
"9ae43cad500350d2637d42a972f11d8b7489879a"}
Message was sent while issue was closed.
Description was changed from ========== [Payments] Fix metric log. BUG=725960 ========== to ========== [Payments] Fix metric log. BUG=725960 Review-Url: https://codereview.chromium.org/2899083005 Cr-Commit-Position: refs/heads/master@{#474387} Committed: https://chromium.googlesource.com/chromium/src/+/9ae43cad500350d2637d42a972f1... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:80001) as https://chromium.googlesource.com/chromium/src/+/9ae43cad500350d2637d42a972f1... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
