Skip to content

Launch tag type parameter for AdSense & GAM#31827

Merged
keithwrightbos merged 1 commit intoampproject:masterfrom
keithwrightbos:launch_ptt
Jan 6, 2021
Merged

Launch tag type parameter for AdSense & GAM#31827
keithwrightbos merged 1 commit intoampproject:masterfrom
keithwrightbos:launch_ptt

Conversation

@keithwrightbos
Copy link
Contributor

Original code change introduced in #30764 wrapped in experiment flag. Launching change.

@amp-owners-bot
Copy link

amp-owners-bot bot commented Jan 6, 2021

Hey @Jiaming-X! These files were changed:

extensions/amp-ad-network-adsense-impl/0.1/amp-ad-network-adsense-impl.js

Hey @jeffkaufman! These files were changed:

extensions/amp-ad-network-adsense-impl/0.1/amp-ad-network-adsense-impl.js
extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js
extensions/amp-ad-network-doubleclick-impl/0.1/test/test-amp-ad-network-doubleclick-impl.js

Copy link
Contributor

@jeffkaufman jeffkaufman left a comment

Choose a reason for hiding this comment

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

Confirmed experiment was neutral, as expected, and code cleanup looks correct

Copy link
Contributor

@lannka lannka left a comment

Choose a reason for hiding this comment

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

for easier rollback, i suggest we first launch it with config only change. then follow up a clean up PR later when things are stable.

Copy link
Contributor

@lannka lannka left a comment

Choose a reason for hiding this comment

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

communicated offline, pure config change can not launch this feature 100%.

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