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

✨ Lentainform. this change is very important #23942

Merged
merged 29 commits into from Oct 2, 2019

Conversation

DenyHF
Copy link
Contributor

@DenyHF DenyHF commented Aug 14, 2019

Hello.
These changes have been lost in previous commits.

Denys Siroshtan added 28 commits April 16, 2019 14:21
This reverts commit cbe9046.
This reverts commit 7ea09a7.
…entainform

# Conflicts:
#	3p/integration.js
#	ads/_config.js
@DenyHF DenyHF changed the title Lentainform. this change is very important ✨ Lentainform. this change is very important Aug 20, 2019
@DenyHF
Copy link
Contributor Author

DenyHF commented Sep 3, 2019

@zhouyx
Hello
Can you approve this request? thank you.

@zhouyx zhouyx requested a review from lannka September 3, 2019 12:26
@zhouyx
Copy link
Contributor

zhouyx commented Sep 3, 2019

Hi @DenyHF Sorry for the delay in review. I am currently traveling.
To @lannka and the ads team.

@DenyHF
Copy link
Contributor Author

DenyHF commented Sep 3, 2019

@zhouyx, thank you for fast answer.
Have a nice traveling.

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.

I would actually suggest you put those logic in your remote JS, for flexibility.

ads/lentainform.js Outdated Show resolved Hide resolved
@lannka lannka merged commit 63a4545 into ampproject:master Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants