-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 error property #3560
Deprecate error property #3560
Conversation
if (arguments.length === 2) { | ||
this._error = error; | ||
} | ||
Ember.deprecate('DS.Model#error has been deprecated please use adapterError instead'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems pretty frequent and i imagine users will disable warnings altogether as their apps will get super noisy. We should probably deprecate once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sense, I will update later tonight.
cd6acb1
to
cdbac46
Compare
@fivetanley updated for only show deprecation once |
come on appveyor... |
return Ember.get(this, 'adapterError'); | ||
} | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets just make this a normal computed property so it can be overridden. Part of the original reason for renaming error to adapterError
was because people were defining an error
attribute on their models. That should still be allowed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, but now that I am thinking, if someone defined error
attribute on the model, the deprecation will not work anyway. Not sure what to do about it. Maybe it is double, but I have no idea how to define a property that can be overridden but will still run some code on get...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If someone defines an error attribute I don't think we should log the deprecation because presumably they want to use the error property as an attribute and are not accidentally trying to reference adapterError
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bmac this is what I assumed. PR updated.
} | ||
return Ember.get(this, 'adapterError'); | ||
} | ||
}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this computed need a dependency on adapterError
or should it be marked volatile?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
off cours... Sorry for the mess with this thing. Should have been an easy one. Updated...
Tested and it works great. 👍 |
@bmac as discussed, here is the deprecation for
error
property