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

Deprecate options #2229

Closed
wants to merge 7 commits into from
Closed

Deprecate options #2229

wants to merge 7 commits into from

Conversation

mmcc
Copy link
Member

@mmcc mmcc commented Jun 3, 2015

  • Added deprecation notice to Component#options()
  • Switch to passing the parent video options to each child as parentOptions
  • Use parentOptions from this.options_ throughout tests / code base

@@ -186,6 +186,8 @@ class Component {
* @return {Object} A NEW object of this.options_ and obj merged
*/
options(obj) {
log.warn('this.options() has been deprecated and will be moved to the constructor in 6.0');
Copy link
Member

Choose a reason for hiding this comment

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

We might want to consider bringing in util-deprecate so warnings are only logged once. Not required for this PR.

@mmcc mmcc force-pushed the deprecate-options branch from 609821b to cd3c6de Compare June 3, 2015 18:03
@mmcc mmcc added this to the v5.0.0 milestone Jun 3, 2015
@@ -76,11 +77,15 @@ class PlaybackRateMenuButton extends MenuButton {
this.player().playbackRate(newRate);
}

playbackRates() {
return (this.options_.playerOptions && this.options_.playerOptions['playbackRates']) || this.options_['playbackRates'];
Copy link
Member

Choose a reason for hiding this comment

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

note: reverse this

@heff
Copy link
Member

heff commented Jun 4, 2015

A few small notes but this looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants