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.

Sign up for GitHub

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

Jump to bottom

🚀 SunMedia: Update amp-ad #38128

Merged
merged 6 commits into from
May 3, 2022
Merged

🚀 SunMedia: Update amp-ad #38128

merged 6 commits into from
May 3, 2022

Conversation

aaronpando
Copy link
Contributor

Update of SunMedia ads

  • CDN entry point and preconnect config to reflect infrastructure migration
  • Initial dimensions and ratio
  • Remove no longer used allowed data params
  • Set clientIdScope and fullWidthHeightRatio for SunMedia

@amp-owners-bot amp-owners-bot bot requested a review from calebcordry April 21, 2022 16:40
@CLAassistant
Copy link

CLAassistant commented Apr 21, 2022 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@calebcordry calebcordry enabled auto-merge (squash) April 21, 2022 21:49
@aaronpando
Copy link
Contributor Author

Hello @calebcordry,

I have noticed that there is a "1 visual change needs revision" in "percy/amphtml"

But I'm pretty sure it has nothing to do with my changes.
What should I do to solve this problem?

Also, there is a unit test failing at ci/circleci

"BentoLightboxGallery preact component Captions should render with captions from caption, alt, and aria-label props"

AssertionError: expected 'Third and fourth img group' to equal 'First img'
at Context. (home/circleci/project/src/bento/components/bento-lightbox-gallery/1.0/test/test-component.js:965:33)

But again, I don't see how my changes affect this component.

Thanks for you help

@aaronpando
Copy link
Contributor Author

Hello @calebcordry,

Can I get an update of the changes?
Could you confirm if I need to modify anything else?

Thanks!

@calebcordry
Copy link
Member

Sorry I thought this one had merged. I went ahead and restarted the flaky test, feel free to ping here if you see them fail again.

@calebcordry calebcordry merged commit d365219 into ampproject:main May 3, 2022
@ampprojectbot
Copy link
Member

Warning: disparity between this PR Percy build and its main build

The Percy build for this PR was approved (either manually by a member of the AMP team, or automatically if there were no visual diffs). However, during a continuous integration step we generated another Percy build using the commit on the main branch that this PR was merged into, and there appears to be a mismatch between the two.

This is possibly an indication of an issue with this pull request, but could also be the result of flakiness. Please inspect the two builds < This PR's Percy build / main commit's Percy build > and determine further action:

  • If the disparity appears to be caused by this PR, please create an bug report or send out a new PR to fix
  • If the disparity appears to be a flake, please @-mention ampproject/wg-approvers in a comment
  • If there is no disparity and this comment was created by mistake, please @-mention ampproject/wg-infra
  • If unsure, @-mention ampproject/wg-approvers

westonruter added a commit that referenced this pull request May 20, 2022
@westonruter
Merge branch 'main' of github.com:ampproject/amphtml into add/amp-meg…
e116f69 
…a-menu-images-validator-spec

* 'main' of github.com:ampproject/amphtml: (90 commits)
  🔥 [Story mediapool] Fix videos from mediapool with `noaudio` don't have audio when reused. (#38216)
  Hide progress bar on the control group of auto advance experiment (#38215)
  ✨ Add Bento Autocomplete Component (#37837)
  🌐 [Story subscription] Subscription localization async (#38204)
  Dable: add new optional parameter "channel" (#38199)
  ✨ [AMP Story Paywall] Enable developers to specify a custom subscriptions page index (#38175)
  SwG Release 0.1.22.217 (#38187)
  amp-script: implements new size limits for sandboxed scripts (#38185)
  🖍  Hide the system layer and progress bar in preview mode (#38163)
  added minItems (#38177)
  Prevent expandTemplate from ReDOSing (#38178)
  Change amp-story-subscriptions attribute name to reflect its flexibility (#38176)
  🐛 [Story Preview] Enable amp-video to play in preview mode (#38149)
  Added the possibility to get page count to story messaging api (#38170)
  SwG Release 0.1.22.216 (#38168)
  Allow @newmuis to update OWNERS files (#38169)
  ✨ Add Richaudience to RTC callout vendors (#38160)
  🚀  SunMedia: Update amp-ad (#38128)
  Remove option to deploy PR artifacts to a static website (#38152)
  added some vars and requests in gfksensic.json (#37722)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@calebcordry calebcordry calebcordry approved these changes

Assignees
No one assigned
Labels
PR use: In Beta / Experimental PR use: In LTS PR use: In Stable
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@aaronpando @CLAassistant @calebcordry @ampprojectbot