Skip to content
This repository has been archived by the owner on Nov 11, 2017. It is now read-only.

Ember.Select .create, .reopen, .extend do not issue deprecation warnings #7

Merged
merged 1 commit into from
Jun 12, 2015

Conversation

bantic
Copy link
Member

@bantic bantic commented Jun 11, 2015

(also fixes type "doest" in some tests)

related PR in ember: emberjs/ember.js#11416

fixes #3

@rwjblue
Copy link
Member

rwjblue commented Jun 11, 2015

Might also want to add one for .extend?

@bantic bantic changed the title Ember.Select .create and .reopen do not issue deprecation warnings Ember.Select .create, .reopen, .extend do not issue deprecation warnings Jun 11, 2015
@bantic
Copy link
Member Author

bantic commented Jun 11, 2015

@rwjblue good idea. done! We do not actually issue any deprecations in Ember for extend or reopen of Ember.Select, only creation. If that seems like an oversight let me know.

Seems worthwhile to cover extend and reopen in this addon regardless.

@rwjblue
Copy link
Member

rwjblue commented Jun 11, 2015

Yes, we should deprecate .extend also. It is very common to put something like:

// app/components/my-special-select.js

export default Ember.Select.extend({
  // special things here
});

@bantic
Copy link
Member Author

bantic commented Jun 12, 2015

@rwjblue The thinking was that as long as we are issuing the deprecation from init, cases where someone extended Ember.Select would still eventually hit that deprecation when the extended class was instantiated later.

Adding a deprecation to extend as well would probably end up with at least 2 deprecation warnings. Putting it only on init seemed like the best way to achieve the goal (discourage Ember.Select use) with minimal noise.

I am happy to add the warnings to extend etc, though, if you think it's ok to have those multiple warnings.

@rwjblue
Copy link
Member

rwjblue commented Jun 12, 2015

Thank you for thinking it through and explaining. I agree with you completely, no need for two deprecations.

rwjblue added a commit that referenced this pull request Jun 12, 2015
`Ember.Select` `.create`, `.reopen`, `.extend` do not issue deprecation warnings
@rwjblue rwjblue merged commit 50ce341 into emberjs:master Jun 12, 2015
@bantic bantic deleted the test-Ember-Select branch June 12, 2015 14:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a test asserting Ember.Select.create works
2 participants