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

Move default options into the constructor #2033

Open
1 of 3 tasks
heff opened this issue Apr 14, 2015 · 3 comments
Open
1 of 3 tasks

Move default options into the constructor #2033

heff opened this issue Apr 14, 2015 · 3 comments
Labels
enhancement needs: discussion Needs a broader discussion pinned Things that stalebot shouldn't close automatically
Milestone

Comments

@heff
Copy link
Member

heff commented Apr 14, 2015

In #1993 (diff) we discussed moving options into the constructor instead of on the prototype. Secondarily removing the this.options() accessor.

For the player we use the prototype options as global options for the player, but this could be done by providing a global options object that overrides the default when the player is initialized.

I'm not sure what impact removing the options() accessor would have, but right now it creates another source of truth, which isn't great. This would force us to be more specific about providing the info from options that we need.

@dmlap @gkatsev, are there places externally where we're relying on the options() getter?

Steps:

  • Deprecate the use of component.options() (v5.0)
  • Move all prototype.options_ uses into the constructor (5.x)
  • Remove component.options() (v.6.0)
@heff heff added enhancement needs: discussion Needs a broader discussion labels Apr 14, 2015
@heff heff added this to the v5.0.0 milestone Apr 14, 2015
This was referenced Apr 21, 2015
@heff
Copy link
Member Author

heff commented May 27, 2015

Instead of attacking this completely in 5.0 I want to start the process by deprecating the use of component.options() and fixing the uses of it over time before removing it in 6.0. I think we use it a lot, especially in plugins, so trying to fix everything for 5.0 would be too much.

@dmlap
Copy link
Member

dmlap commented May 28, 2015

People are definitely using the options() getter externally but I support deprecating it to avoid confusion over options versus properties. Pushing actual removal to 6.0 seems like a good move.

@heff
Copy link
Member Author

heff commented Jun 5, 2015

#2229 checked the first box for deprecating options(). I added a check list item for moving prototype options into the constructor. Pushing this to 5.1.

@heff heff modified the milestones: v5.1.0, v5.0.0 Jun 5, 2015
@heff heff mentioned this issue Oct 7, 2015
@gkatsev gkatsev added the pinned Things that stalebot shouldn't close automatically label Jul 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement needs: discussion Needs a broader discussion pinned Things that stalebot shouldn't close automatically
Projects
None yet
Development

No branches or pull requests

3 participants