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

✨ Add Highfivve as AMP RTC vendor #33254

Merged
merged 2 commits into from
Mar 18, 2021

Conversation

muuki88
Copy link
Contributor

@muuki88 muuki88 commented Mar 15, 2021

This pull requests adds AMP RTC vendor Highfivve to the list of AMP RTC vendors.

Example integration

<amp-ad
  width="320"
  height="50"
  type="network-foo"
  data-slot="/1234/5678"
  rtc-config='{
    "vendors": {
      "highfivve": {"TAG_ID": "1", "ACCOUNT_ID":"my-account"}
      },
    "timeoutMillis": 750}'
>
</amp-ad>

Follow up of the mess I created in #33203 . Sorry for the inconvenience 🙇‍♂️

@calebcordry
Copy link
Member

Sorry we have some flaky tests, could you try rebase or merging latest master. It should fix the tests. Let me know if you run into problems.

@muuki88
Copy link
Contributor Author

muuki88 commented Mar 18, 2021

Looks fine now ❤️

When this gets merged, how long will it take to be available on AMP pages?

@calebcordry calebcordry merged commit 1c83fb9 into ampproject:master Mar 18, 2021
@calebcordry
Copy link
Member

Thanks for contributing! You can test using nightly tomorrow. It will hit beta next week, then prod the following week. You can find more details at https://amp.dev/documentation/guides-and-tutorials/learn/spec/release-schedule/

@muuki88 muuki88 deleted the add-highfivve branch March 18, 2021 19:57
@muuki88
Copy link
Contributor Author

muuki88 commented Mar 18, 2021

Thanks for your help ❤️

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.

None yet

3 participants