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

[CLEANUP beta] Remove beforeObserver family from the public API #11796

Merged
merged 1 commit into from
Jul 17, 2015

Conversation

cibernox
Copy link
Contributor

  • family inclides Ember.beforeObserver, Ember.addBeforeObserver,
    Ember.removeBeforeObserver, Ember.beforeObserversFor,
    Ember._suspendBeforeObserver, Ember._suspendBeforeObservers
  • I noticed that beforeObserversFor, _suspendBeforeObserver and _suspendBeforeObserves
    were not used at all, not even internally. I've also checked for
    usages of it in ember-data, liquid-fire and other "official" porjects.
    Nothing. So those two are removed completely.
  • For symetry, I've also removed the Function.prototype.observesBefore
    counterpart.

@private
*/
FunctionPrototype.observesBefore = function () {
var watched = [];
Copy link
Member

Choose a reason for hiding this comment

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

this appears to have have been deprecated.

Copy link
Member

Choose a reason for hiding this comment

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

Do you mean, it was not deprecated?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I mean it did no spew a deprecation warning when used. Is there a sibling PR that introduces that deprecation.

Copy link
Member

Choose a reason for hiding this comment

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

Ya, good catch.

@cibernox - We need to land a deprecation for this in release before removing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There isn't. I'll create one. Put this on hold in the meanwhile

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rwjblue
Copy link
Member

rwjblue commented Jul 17, 2015

Needs a rebase now that #11798 is merged.

* family inclides `Ember.beforeObserver`, `Ember.addBeforeObserver`,
  `Ember.removeBeforeObserver`, `Ember.beforeObserversFor`,
  `Ember._suspendBeforeObserver`, `Ember._suspendBeforeObservers`

* I noticed that `beforeObserversFor`, `_suspendBeforeObserver` and `_suspendBeforeObserves`
  were not used at all, not even internally. I've also checked for
  usages of it in ember-data, liquid-fire and other "official" porjects.
  Nothing. So those two are removed completely.

* For symetry, I've also removed the `Function.prototype.observesBefore`
  counterpart.
@cibernox
Copy link
Contributor Author

Rebased

stefanpenner added a commit that referenced this pull request Jul 17, 2015
[CLEANUP beta] Remove beforeObserver family from the public API
@stefanpenner stefanpenner merged commit 3ab4857 into emberjs:master Jul 17, 2015
@cibernox cibernox deleted the remove_before_observers branch July 17, 2015 19:58
@denzo
Copy link

denzo commented Aug 15, 2015

SortableMixin from ember-legacy-controllers breaks as it uses beforeObserver :(

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.

4 participants