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

✨Adds smoothing to amp-orientation-observer #24606

Merged
merged 5 commits into from Sep 30, 2019
Merged

✨Adds smoothing to amp-orientation-observer #24606

merged 5 commits into from Sep 30, 2019

Conversation

wassgha
Copy link
Contributor

@wassgha wassgha commented Sep 18, 2019 •
edited

Closes #19326 , Closes #24400

Changes

  • Adds a configurable smoothing of the alpha/beta/gamma values (the configurable value is the subset size used to generate the moving average)
  • Dynamically reads the rest values of alpha/beta/gamma instead of defaulting to 0
  • Clamps the values to the provided ranges
  • Adds the required validator tests

@Gregable
Copy link
Member

Other than my one optional comment/question, validator changes LGTM.

twifkak
twifkak approved these changes Sep 25, 2019
View reviewed changes
Copy link
Member

@twifkak twifkak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay! LGTM modulo one suggestion.

extensions/amp-orientation-observer/0.1/test/validator-amp-orientation-observer.html Show resolved Hide resolved
@amp-owners-bot
Copy link

Hey @ampproject/wg-caching, these files were changed:

  • extensions/amp-orientation-observer/0.1/test/validator-amp-orientation-observer.html
  • extensions/amp-orientation-observer/0.1/test/validator-amp-orientation-observer.out
  • extensions/amp-orientation-observer/validator-amp-orientation-observer.protoascii

@wassgha wassgha merged commit c543dd9 into ampproject:master Sep 30, 2019
@wassgha wassgha deleted the smoothing-amp-orientation branch September 30, 2019 16:35
@wassgha wassgha mentioned this pull request Oct 2, 2019
Closed
amaltas added a commit that referenced this pull request Oct 2, 2019
@amaltas
Validator rollup 401@960 (#24857)
4c32922 
* cl/272111029 Replace some assert.fail calls with goog.assert.fail.

* cl/272139712 Revision bump for #24606

* cl/272281163 Revision bump for #24572

* cl/272290443 Revision bump for #24839
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@twifkak twifkak twifkak approved these changes

@Gregable Gregable Gregable left review comments

@honeybadgerdontcare honeybadgerdontcare honeybadgerdontcare left review comments

@sparhami sparhami sparhami approved these changes

@nainar nainar Awaiting requested review from nainar

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
6 participants
@wassgha @Gregable @twifkak @honeybadgerdontcare @sparhami @googlebot