-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updating amp-analytics comScore parameters #11054
Conversation
acorretti-comscore
commented
Aug 23, 2017
- Add pageViewId (cs_pv)
- Add clientId(comScore) (c12)
* Add pageViewId (cs_pv) * Add clientId(comScore) (c12)
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll also need to update the tests here: https://github.com/ampproject/amphtml/blob/master/extensions/amp-analytics/0.1/test/vendor-requests.json
Thanks @avimehta, I've updated the tests accordingly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that you had authored the original comscore tag. Based on that, this PR looks good to me.
@acorretti You don't need to squash the commits but it looks like CLA check is not passing yet. Can you go through the GoogleBot comment and see if you can resolve the CLA issue? |
@avimehta I did fulfill the CLA agreement in the name of comScore, Inc. to be signed by my manager Thomas Pottjegort. He signed it earlier this evening, but I don't know how to tell the GoogleBot about that. |
Let's see if the CLA bot can find our CLA now. |
CLAs look good, thanks! |
@acorretti Thanks for the PR! |