Skip to content
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

Added bhType and all site traffic #3828

Closed
wants to merge 2 commits into from

Conversation

ColombiaAnalytics
Copy link
Contributor

modified 2 files

mukultanwar14 and others added 2 commits April 25, 2016 12:41
Changes in examples/analytics.amp.html extensions/amp-analytics/amp-analytics.md
@avimehta
Copy link
Contributor

@ColombiaAnalytics Please resolve the branch conflicts before I can proceed with the review.

The diff should show only the new parts that you are adding in this pull request. Currently the PR shows that you are trying to add colanalytics but that was already added a few months ago.

@ColombiaAnalytics
Copy link
Contributor Author

Hi,

Can you help me in this. Reason I tried to remove it from pull request couldn't find any option. Created a new branch rebased master and cherry-pick but it did not work .

Thanks & Regards,

Mukul Tanwar
9717359710


From: Avi Mehta notifications@github.com
Sent: Thursday, June 30, 2016 12:55:41 AM
To: ampproject/amphtml
Cc: Mukul Tanwar; Mention
Subject: Re: [ampproject/amphtml] Added bhType and all site traffic (#3828)

@ColombiaAnalyticshttps://github.com/ColombiaAnalytics Please resolve the branch conflicts before I can proceed with the review.

The diff should show only the new parts that you are adding in this pull request. Currently the PR shows that you are trying to add colanalytics but that was already added a few months ago.

You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/3828#issuecomment-229461598, or mute the threadhttps://github.com/notifications/unsubscribe/ARWRN-jnCxElkGGWHfpUPLqBXkofRWU7ks5qQsa1gaJpZM4JBH9Z.

The hunt is on for young creative and media minds under 30 years. Brand Equity Young Spikes 2016 competition. Winners get a free trip to Singapore to compete at Spikes Asia 2016. www.etbrandequity.com/youngspikes

@avimehta
Copy link
Contributor

Can you reply here and tell me what you want to add to your config?

If you want to do it, It might be easier for you to start from beginning and make the changes in a new clone of the repo.

@ColombiaAnalytics
Copy link
Contributor Author

Hi,

My changes are simple need to just introduce t more query parameters in the url.

Thanks & Regards,

Mukul Tanwar
9717359710


From: Avi Mehta notifications@github.com
Sent: Thursday, June 30, 2016 11:05:04 PM
To: ampproject/amphtml
Cc: Mukul Tanwar; Mention
Subject: Re: [ampproject/amphtml] Added bhType and all site traffic (#3828)

Can you reply here and tell me what you want to add to your config?

If you want to do it, It might be easier for you to start from beginning and make the changes in a new clone of the repo.

You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/3828#issuecomment-229732040, or mute the threadhttps://github.com/notifications/unsubscribe/ARWRN1qS9qH3HrBt-ac9IZiAAAMlC6PPks5qQ_5IgaJpZM4JBH9Z.

The hunt is on for young creative and media minds under 30 years. Brand Equity Young Spikes 2016 competition. Winners get a free trip to Singapore to compete at Spikes Asia 2016. www.etbrandequity.com/youngspikes

@ColombiaAnalytics
Copy link
Contributor Author

Hi,
My changes are just to introduce 2 query Param in the url

@avimehta
Copy link
Contributor

i understand, Can you paste the code here? I'll create a pull request for you which you can review and approve.

@ColombiaAnalytics
Copy link
Contributor Author

Thanks ,
The changes are in url as in below file the change is in "pagereview" introduces
&val_101=$id and &val_120=3
In file ---> extensions/amp-analytics/0.1/test/vendor-requests.json

"colanalytics": {
"host": "https://ase.clmbtech.com",
"base": "https://ase.clmbtech.com/message",
"pageview": "https://ase.clmbtech.com/message?cid=$id**&val_101=$id**&val_101=_canonical_path_&ch=_canonical_host_&uuid=$uid&au=$authors&zo=$zone&sn=$sponsorName&ct=$contentType&st=_scroll_top_&sh=_scroll_height_&dct=$decayTime&tet=_total_engaged_time_&dr=_document_referrer_&plt=_page_load_time_&val_108=_title_**&val_120=3**"
},

The change here is in "colanalytics" again added
&val_101=$id and &val_120=3
In file ---> extensions/amp-analytics/0.1/vendors.js

'colanalytics': {
'requests': {
'host': 'https://ase.clmbtech.com',
'base': '${host}/message',
'pageview': '${base}?cid=${id}' +
'&val_101=${id}' +
'&val_101=${canonicalPath}' +
'&ch=${canonicalHost}' +
'&uuid=${uid}' +
'&au=${authors}' +
'&zo=${zone}' +
'&sn=${sponsorName}' +
'&ct=${contentType}' +
'&st=${scrollTop}' +
'&sh=${scrollHeight}' +
'&dct=${decayTime}' +
'&tet=${totalEngagedTime}' +
'&dr=${documentReferrer}' +
'&plt=${pageLoadTime}' +
'&val_108=${title}' +
'&val_120=3',

@avimehta
Copy link
Contributor

@ColombiaAnalytics can you review #3849? I'll merge it once you give your okay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants