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

✨ SwG: Propensity to Subscribe in AMP #26003

Merged
merged 22 commits into from
Jan 3, 2020

Conversation

mborof
Copy link
Contributor

@mborof mborof commented Dec 12, 2019

Implements Propensity to Subscribe in AMP

@zhouyx
Copy link
Contributor

zhouyx commented Dec 20, 2019

We are in the middle of migrating all vendor config from js file to json file. The idea is that the config doesn't need to be compiled into the amp-analytics.js, instead AMP can load them lazily. Please also check in a json version of the vendor config file. You can do so by running gulp generate-vendor-jsons

Please refer to #26050 for more information on the migration process.

Copy link
Contributor

@jpettitt jpettitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved subject to confirming with the amp analytics team that "propensity" is the right name and they they don't want it prefixed by "google-" or "subscriptions-" or something similar.

@mborof
Copy link
Contributor Author

mborof commented Jan 2, 2020

Renamed it subscriptions-propensity for consistency with the rest of SwG.

@zhouyx
Copy link
Contributor

zhouyx commented Jan 2, 2020

subscriptions-propensity SGTM. We can recommend all future vendors who work closely with swg to add prefix subscripts-. Thank you

Copy link
Contributor

@zhouyx zhouyx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vendor Config LGTM

@dvoytenko dvoytenko merged commit 8ef8f17 into ampproject:master Jan 3, 2020
sparhami pushed a commit to sparhami/amphtml that referenced this pull request Jan 3, 2020
* Propensity to Subscribe in AMP

* fix types in AMP

* minor improvements, fix tests

* use getSku, add propensity tests

* generate new file format

* subscriptions-propensity

* finish rename

* redo data parameter

* change formatting

* adjust expectations for new format

* rename file

* remove comments

* change change variable coding
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.

6 participants