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

Fix amp-video amp_video_quality parameter name for cached sources. #33227

Merged

Conversation

gmajoulet
Copy link
Contributor

Fixes #33225

@erwinmombay erwinmombay merged commit c8b2faf into ampproject:master Mar 12, 2021
ampprojectbot pushed a commit that referenced this pull request Mar 12, 2021
ampprojectbot pushed a commit that referenced this pull request Mar 12, 2021
ampprojectbot pushed a commit that referenced this pull request Mar 12, 2021
@kristoferbaxter
Copy link
Contributor

Can we add a more stringent test that validates the url parameter name?

@Gregable
Copy link
Member

Can we add a more stringent test that validates the url parameter name?

It would be very difficult to write an integration test crossing the boundary between amphtml codebase and amp cache codebase, especially since the latter isn't even in github. If you mean a test that verifies the param is amp_video_quality, that appears to be here but that test wouldn't have caught this.

westonruter added a commit to westonruter/amphtml that referenced this pull request Mar 15, 2021
* 'master' of github.com:ampproject/amphtml: (536 commits)
  ✨ Send extension versions from AMPHTML ads to FIE  (ampproject#33190)
  Update amp-autocomplete test output file (ampproject#33216)
  ✨ Changing Inline Page Attachment Button UI  (ampproject#33142)
  ✨Launch no-signing (ampproject#33229)
  ♻️ Add logic for story ad page manager. (ampproject#33201)
  Migrate Bento components to V1 (ampproject#32974)
  📦 Update dependency mocha to v8.3.2 (ampproject#33234)
  📦 Update build system devDependencies (ampproject#33178)
  🧪 [amp-consent] Turn on TCF PostMessage Proxy API Experiment (ampproject#33196)
  Fix amp-video amp_video_quality parameter name. (ampproject#33227)
  🏗 Guard access to `BASH_ENV` during CircleCI push builds (ampproject#33231)
  🏗 Clean up CircleCI merge SHA logic (ampproject#33215)
  📦 Update dependency karma to v6.1.2 (ampproject#33151)
  🏗 Ensure up-to-date and clean `package-lock.json` before running any CI job (ampproject#33223)
  Ensure that pre-stubbing always happens before the first element is upgraded (ampproject#33212)
  🏗 Restore `package-lock.json` to `lockfileVersion = 1` (ampproject#33222)
  Fixed text align of buttons (ampproject#33217)
  [amp-story] ♿ Add label for next story and add i18n for pagination buttons (ampproject#33205)
  [amp-story-player] 📖  Rename skip-next control to skip-to-next (ampproject#33164)
  🚀 Report cls-fcp and cls-vis to compare with cls (ampproject#33207)
  ...
muuki88 pushed a commit to highfivve/amphtml that referenced this pull request Mar 15, 2021
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.

amp-video produce invalid cached source URLs for Stories
5 participants