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

✨[amp-ad smartadserver] Include consent data in iframe name #39965

Merged
merged 4 commits into from
Apr 23, 2024

Conversation

eszponder
Copy link
Contributor

No description provided.

@CLAassistant
Copy link

CLAassistant commented Apr 17, 2024

CLA assistant check
All committers have signed the CLA.

@amp-owners-bot amp-owners-bot bot requested a review from powerivq April 17, 2024 07:59
Copy link

amp-owners-bot bot commented Apr 17, 2024

Hey @smart-adserver! These files were changed:

extensions/amp-ad-network-smartadserver-impl/0.1/amp-ad-network-smartadserver-impl.js
extensions/amp-ad-network-smartadserver-impl/0.1/test/test-amp-ad-network-smartadserver-impl.js

@eszponder
Copy link
Contributor Author

Hello @powerivq, thank you for the review and approve. I noticed that some test is failing and I'm not sure why, could you please take a look on it and give me some hint? All ut from our extensions are passing. Thanks in advance for your time.

@powerivq
Copy link
Contributor

@eszponder I would suggest rebasing against the latest main branch.

@powerivq
Copy link
Contributor

@eszponder I am wondering what we are doing here? The PR is now empty.

Sadr 5675 - include consent for GPT
@eszponder eszponder reopened this Apr 23, 2024
@eszponder
Copy link
Contributor Author

Hello @powerivq I was doing rebase as you suggested and I had some conflicts so I removed my commits for a while. Now the PR is updated

@powerivq powerivq merged commit 199d1b3 into ampproject:main Apr 23, 2024
57 checks passed
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