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

Promote CID related code to core (out of amp-analytics) #4898

Closed
lannka opened this issue Sep 9, 2016 · 2 comments
Closed

Promote CID related code to core (out of amp-analytics) #4898

lannka opened this issue Sep 9, 2016 · 2 comments

Comments

@lannka
Copy link
Contributor

lannka commented Sep 9, 2016

Motivation:
Many extensions need CID hence depend on amp-analyitcs. We could remove the dependencies once get CID promoted to core.

  • amp-user-notification
  • amp-ad
  • amp-access
  • Any place needs CLIENT_ID in URL substitution

And, starting recently <amp-analytics> is blocked by ad blocker: #4896 .

Work to be done
This would need to move the following code to core, the cons is increased core size, an estimate is listed below:

File to promote Raw file size Minified size
cid-impl.js 12k 3.7k
crypto-impl.js 3.6k 2.0k
closure/sha384.js 9.2k 9.2k

The real size after closure complier should be smaller.
And we can late load the sha384.js lib as tracked here: #3888

@lannka
Copy link
Contributor Author

lannka commented Feb 10, 2017

One more PR needed: move CID code to core. After that, we should change READMEs of those extensions that previously dependent on amp-analytics to generate CID.

@lannka
Copy link
Contributor Author

lannka commented Jun 4, 2017

The remaining work is tracked as a Great First Issue here: #9713

@lannka lannka closed this as completed Jun 4, 2017
@rudygalfi rudygalfi removed this from Feature Backlog in Analytics Jun 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants