New issue
Advanced search Search tips

Issue 753178 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

[Payments] Move metrics to PaymentRequest.Events

Project Member Reported by ma...@chromium.org, Aug 8 2017

Issue description

Bitfield (UMA/UKM) allows for more flexible querying. 

Move some of the existing metrics there, and add new ones.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8b17a8ded1efd5f7b490e53873270e769698d591

commit 8b17a8ded1efd5f7b490e53873270e769698d591
Author: Mathieu Perreault <mathp@chromium.org>
Date: Tue Aug 08 15:33:46 2017

[Payments] Move requested information metrics to PaymentRequest.Events

TBR=holte

Bug:  753178 
Test: existing
Change-Id: I26e82c61af8a12394f77236916702636a8d5b944
Reviewed-on: https://chromium-review.googlesource.com/604928
Commit-Queue: Mathieu Perreault <mathp@chromium.org>
Reviewed-by: Ganggui Tang <gogerald@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492631}
[modify] https://crrev.com/8b17a8ded1efd5f7b490e53873270e769698d591/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestContactDetailsAndFreeShippingTest.java
[modify] https://crrev.com/8b17a8ded1efd5f7b490e53873270e769698d591/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestContactDetailsTest.java
[modify] https://crrev.com/8b17a8ded1efd5f7b490e53873270e769698d591/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestEmailAndFreeShippingTest.java
[modify] https://crrev.com/8b17a8ded1efd5f7b490e53873270e769698d591/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestEmailAndPhoneTest.java
[modify] https://crrev.com/8b17a8ded1efd5f7b490e53873270e769698d591/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestEmailTest.java
[modify] https://crrev.com/8b17a8ded1efd5f7b490e53873270e769698d591/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestFreeShippingTest.java
[modify] https://crrev.com/8b17a8ded1efd5f7b490e53873270e769698d591/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestJourneyLoggerTest.java
[modify] https://crrev.com/8b17a8ded1efd5f7b490e53873270e769698d591/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestMetricsTest.java
[modify] https://crrev.com/8b17a8ded1efd5f7b490e53873270e769698d591/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestNameAndFreeShippingTest.java
[modify] https://crrev.com/8b17a8ded1efd5f7b490e53873270e769698d591/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestNameTest.java
[modify] https://crrev.com/8b17a8ded1efd5f7b490e53873270e769698d591/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestNoShippingTest.java
[modify] https://crrev.com/8b17a8ded1efd5f7b490e53873270e769698d591/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestPhoneAndFreeShippingTest.java
[modify] https://crrev.com/8b17a8ded1efd5f7b490e53873270e769698d591/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestPhoneTest.java
[modify] https://crrev.com/8b17a8ded1efd5f7b490e53873270e769698d591/chrome/browser/ui/views/payments/payment_request_journey_logger_browsertest.cc
[delete] https://crrev.com/dd15fe6331e00a02586b79be48405d6fb1f30a0a/chrome/browser/ui/views/payments/payment_request_requested_information_metric_browsertest.cc
[modify] https://crrev.com/8b17a8ded1efd5f7b490e53873270e769698d591/chrome/test/BUILD.gn
[modify] https://crrev.com/8b17a8ded1efd5f7b490e53873270e769698d591/components/payments/core/journey_logger.cc
[modify] https://crrev.com/8b17a8ded1efd5f7b490e53873270e769698d591/components/payments/core/journey_logger.h
[modify] https://crrev.com/8b17a8ded1efd5f7b490e53873270e769698d591/components/payments/core/journey_logger_unittest.cc
[modify] https://crrev.com/8b17a8ded1efd5f7b490e53873270e769698d591/tools/metrics/histograms/histograms.xml

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/779db4d7ea838f7cf592e700e7d89cc30cf86c93

commit 779db4d7ea838f7cf592e700e7d89cc30cf86c93
Author: Mathieu Perreault <mathp@chromium.org>
Date: Wed Aug 09 12:57:11 2017

[Payments] Remove obsolete CheckoutFunnel metrics.

Initiated, Shown, SkippedShow, Completed, ReceivedInstrumentDetails,
PayClicked are all covered undered PaymentRequest.Events, which is more
flexible.

TBR=holte

Bug:  753178 
Change-Id: I2f6c12952c846ee3bae2c673c8400f50238cb98b
Reviewed-on: https://chromium-review.googlesource.com/606729
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Ganggui Tang <gogerald@chromium.org>
Commit-Queue: Mathieu Perreault <mathp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492957}
[modify] https://crrev.com/779db4d7ea838f7cf592e700e7d89cc30cf86c93/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestMetricsTest.java
[modify] https://crrev.com/779db4d7ea838f7cf592e700e7d89cc30cf86c93/chrome/browser/ui/views/payments/payment_request_completion_status_metrics_browsertest.cc
[modify] https://crrev.com/779db4d7ea838f7cf592e700e7d89cc30cf86c93/chrome/browser/ui/views/payments/payment_request_journey_logger_browsertest.cc
[modify] https://crrev.com/779db4d7ea838f7cf592e700e7d89cc30cf86c93/components/payments/core/journey_logger.cc
[modify] https://crrev.com/779db4d7ea838f7cf592e700e7d89cc30cf86c93/components/payments/core/journey_logger.h
[modify] https://crrev.com/779db4d7ea838f7cf592e700e7d89cc30cf86c93/tools/metrics/histograms/histograms.xml

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/44df24821ae79a205cf9133764f036191a8d24dc

commit 44df24821ae79a205cf9133764f036191a8d24dc
Author: Mathieu Perreault <mathp@chromium.org>
Date: Thu Aug 10 00:37:05 2017

[Payments] Remove metrics that are now included in PaymentRequest.Events

* Removed PaymentRequest.CanMakePayment* metrics
* Removed PaymentRequest.UserDid[Not]HaveCompleteSuggestionsForEverything
* Adjusted some suffixes in histograms.xml that had been missed.
* General code cleanup.

Bug:  753178 
Test: browser_tests, components_unittests
Change-Id: I8d4c8e4589382160d854d77306173bed78e7eb4b
Reviewed-on: https://chromium-review.googlesource.com/608310
Commit-Queue: Mathieu Perreault <mathp@chromium.org>
Reviewed-by: Ganggui Tang <gogerald@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493190}
[modify] https://crrev.com/44df24821ae79a205cf9133764f036191a8d24dc/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestCanMakePaymentMetricsTest.java
[modify] https://crrev.com/44df24821ae79a205cf9133764f036191a8d24dc/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestJourneyLoggerTest.java
[modify] https://crrev.com/44df24821ae79a205cf9133764f036191a8d24dc/chrome/browser/ui/views/payments/payment_request_can_make_payment_metrics_browsertest.cc
[modify] https://crrev.com/44df24821ae79a205cf9133764f036191a8d24dc/chrome/browser/ui/views/payments/payment_request_completion_status_metrics_browsertest.cc
[modify] https://crrev.com/44df24821ae79a205cf9133764f036191a8d24dc/chrome/browser/ui/views/payments/payment_request_journey_logger_browsertest.cc
[modify] https://crrev.com/44df24821ae79a205cf9133764f036191a8d24dc/components/payments/core/journey_logger.cc
[modify] https://crrev.com/44df24821ae79a205cf9133764f036191a8d24dc/components/payments/core/journey_logger.h
[modify] https://crrev.com/44df24821ae79a205cf9133764f036191a8d24dc/components/payments/core/journey_logger_unittest.cc
[modify] https://crrev.com/44df24821ae79a205cf9133764f036191a8d24dc/tools/metrics/histograms/histograms.xml

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/627b97c42df6ba7748c1ec274361f37a469c7ad7

commit 627b97c42df6ba7748c1ec274361f37a469c7ad7
Author: Mathieu Perreault <mathp@chromium.org>
Date: Sat Aug 12 00:44:22 2017

[Payments] Log the payment methods requested by the merchant.

Will log into PaymentRequest.Events. Categories are Basic Card, Google
payment method and other payment methods.

Bug:  753178 
Test: browser_tests, components_unittests
Change-Id: I39187f7b49301622942a5fe4a8b820e6c9738bbb
Reviewed-on: https://chromium-review.googlesource.com/611953
Reviewed-by: Ganggui Tang <gogerald@chromium.org>
Reviewed-by: mahmadi (Moe) <mahmadi@chromium.org>
Commit-Queue: Mathieu Perreault <mathp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493910}
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/chrome/android/java/src/org/chromium/chrome/browser/payments/JourneyLogger.java
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestCanMakePaymentMetricsTest.java
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestContactDetailsAndFreeShippingTest.java
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestContactDetailsTest.java
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestEmailAndFreeShippingTest.java
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestEmailAndPhoneTest.java
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestEmailTest.java
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestFreeShippingTest.java
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestJourneyLoggerTest.java
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestMetricsTest.java
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestNameAndFreeShippingTest.java
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestNameTest.java
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestNoShippingTest.java
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestPhoneAndFreeShippingTest.java
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestPhoneTest.java
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/chrome/browser/payments/android/journey_logger_android.cc
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/chrome/browser/payments/android/journey_logger_android.h
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/chrome/browser/ui/views/payments/payment_request_journey_logger_browsertest.cc
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/components/payments/content/payment_request.cc
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/components/payments/content/payment_request_spec.cc
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/components/payments/content/payment_request_spec.h
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/components/payments/core/journey_logger.cc
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/components/payments/core/journey_logger.h
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/components/payments/core/journey_logger_unittest.cc
[modify] https://crrev.com/627b97c42df6ba7748c1ec274361f37a469c7ad7/ios/chrome/browser/ui/payments/payment_request_manager.mm

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/49a97e5199406538b87f7f47e873fb0bc0f182a6

commit 49a97e5199406538b87f7f47e873fb0bc0f182a6
Author: Mathieu Perreault <mathp@chromium.org>
Date: Tue Aug 15 01:51:36 2017

[Payments] Replace SelectedPaymentMethod histogram with PaymentRequest.Events

Deprecates the SelectedPaymentMethod histogram in favor of an event in
PaymentRequest.Events.

TBR=holte

Bug:  753178 
Test: browser_tests, components_unittests
Change-Id: I0c8b24bc76562ec7917e5582d795e67012b2d027
Reviewed-on: https://chromium-review.googlesource.com/612753
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Reviewed-by: mahmadi (Moe) <mahmadi@chromium.org>
Reviewed-by: Ganggui Tang <gogerald@chromium.org>
Commit-Queue: Mathieu Perreault <mathp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494299}
[modify] https://crrev.com/49a97e5199406538b87f7f47e873fb0bc0f182a6/chrome/android/java/src/org/chromium/chrome/browser/payments/JourneyLogger.java
[modify] https://crrev.com/49a97e5199406538b87f7f47e873fb0bc0f182a6/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
[modify] https://crrev.com/49a97e5199406538b87f7f47e873fb0bc0f182a6/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestCanMakePaymentMetricsTest.java
[modify] https://crrev.com/49a97e5199406538b87f7f47e873fb0bc0f182a6/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestContactDetailsAndFreeShippingTest.java
[modify] https://crrev.com/49a97e5199406538b87f7f47e873fb0bc0f182a6/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestContactDetailsTest.java
[modify] https://crrev.com/49a97e5199406538b87f7f47e873fb0bc0f182a6/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestEmailTest.java
[modify] https://crrev.com/49a97e5199406538b87f7f47e873fb0bc0f182a6/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestJourneyLoggerTest.java
[modify] https://crrev.com/49a97e5199406538b87f7f47e873fb0bc0f182a6/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestMetricsTest.java
[modify] https://crrev.com/49a97e5199406538b87f7f47e873fb0bc0f182a6/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestNameTest.java
[modify] https://crrev.com/49a97e5199406538b87f7f47e873fb0bc0f182a6/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestPhoneAndFreeShippingTest.java
[modify] https://crrev.com/49a97e5199406538b87f7f47e873fb0bc0f182a6/chrome/browser/payments/android/journey_logger_android.cc
[modify] https://crrev.com/49a97e5199406538b87f7f47e873fb0bc0f182a6/chrome/browser/payments/android/journey_logger_android.h
[modify] https://crrev.com/49a97e5199406538b87f7f47e873fb0bc0f182a6/chrome/browser/ui/views/payments/payment_request_journey_logger_browsertest.cc
[modify] https://crrev.com/49a97e5199406538b87f7f47e873fb0bc0f182a6/components/payments/content/payment_request.cc
[modify] https://crrev.com/49a97e5199406538b87f7f47e873fb0bc0f182a6/components/payments/core/journey_logger.cc
[modify] https://crrev.com/49a97e5199406538b87f7f47e873fb0bc0f182a6/components/payments/core/journey_logger.h
[modify] https://crrev.com/49a97e5199406538b87f7f47e873fb0bc0f182a6/ios/chrome/browser/ui/payments/payment_request_manager.mm
[modify] https://crrev.com/49a97e5199406538b87f7f47e873fb0bc0f182a6/tools/metrics/histograms/histograms.xml

Comment 6 by aluo@chromium.org, Aug 23 2017

Labels: chromium-waterfall
The PaymentRequestFreeShippingTest have been failing since at least 62.0.3187.0

I 1334.972s run_tests_on_device(06ab5af7003bede8)    1) testPayWithRender(org.chromium.chrome.browser.payments.PaymentRequestFreeShippingTest)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    org.chromium.chrome.test.util.RenderTestRule$RenderTestException: RenderTest Mismatches for: unmask
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.chromium.chrome.test.util.RenderTestRule.finished(RenderTestRule.java:208)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.junit.a.g.access$400(TestWatcher.java:46)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.junit.a.h.evaluate(TestWatcher.java:64)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.junit.a.e.evaluate(RunRules.java:20)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.chromium.base.test.BaseJUnit4ClassRunner.runChild(BaseJUnit4ClassRunner.java:175)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.chromium.base.test.BaseJUnit4ClassRunner.runChild(BaseJUnit4ClassRunner.java:40)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.junit.runners.e.run(ParentRunner.java:290)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.junit.runners.c.a(ParentRunner.java:71)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.junit.runners.d.evaluate(ParentRunner.java:268)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.chromium.base.test.BaseJUnit4ClassRunner.run(BaseJUnit4ClassRunner.java:164)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.junit.runners.Suite.runChild(Suite.java:128)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.junit.runners.Suite.runChild(Suite.java:27)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.junit.runners.e.run(ParentRunner.java:290)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.junit.runners.c.a(ParentRunner.java:71)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.junit.runners.d.evaluate(ParentRunner.java:268)
I 1334.972s run_tests_on_device(06ab5af7003bede8)    	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
I 1334.973s run_tests_on_device(06ab5af7003bede8)    	at org.junit.runner.e.a(JUnitCore.java:137)
I 1334.973s run_tests_on_device(06ab5af7003bede8)    	at android.support.test.internal.runner.k.a(TestExecutor.java:3115)
I 1334.973s run_tests_on_device(06ab5af7003bede8)    	at android.support.test.runner.a.onStart(AndroidJUnitRunner.java:262)
I 1334.973s run_tests_on_device(06ab5af7003bede8)    	at org.chromium.base.test.BaseChromiumAndroidJUnitRunner.onStart(BaseChromiumAndroidJUnitRunner.java:83)
I 1334.973s run_tests_on_device(06ab5af7003bede8)    	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1701)

Please fix, thanks.

Comment 7 by ma...@chromium.org, Sep 5 2017

Status: Fixed (was: Started)
We are done for now. All required events have been moved.

Sign in to add a comment