prefix private methods with underscore - store.js #3254
Conversation
|
My tests pass sporadically. I get some issues relating to semaphore every now and then. I'm not sure what to do about that. |
| @private | ||
| @param {InternalModel} internalModel | ||
| */ | ||
| recordWasError: function(internalModel) { | ||
| _recordWasError: function(internalModel) { | ||
| Ember.deprecate('_recordWasError was documented as private and will be removed in the next version of Ember Data.'); |
There was a problem hiding this comment.
We want to keep the underscore method working, and create a new method called recordWasError that calls the previous one:
recordWasError: function(internalModel) {
Ember.deprecate('message');
return this._recordWasError(internalModel);
}
|
@gevious It seems my instructions weren't clear, sorry. When we deprecate a method, we still want people to be able to call it until it gets removed. I left an example here https://github.com/emberjs/data/pull/3254/files#r31911784 |
|
Ah, yes. I clearly wasn't thinking this morning. What should I do with the tests, @fivetanley. I've updated them for the new methods. Should I create parallel tests for the deprecated ones? Ie should I have test cases for the deprecated methods along with the new ones? |
|
I've kept the tests the same, but reinserted the deprecated methods. This means the deprecated methods aren't used in the test suite, but the new private methods are.If the new api is used internally, this shouldn't cause any problems going forward. |
| return this.findRecord(modelName, id, preload); | ||
| }, | ||
| findById: function(modelName, id, preload) { | ||
| Ember.deprecate('findById was documented as private and will be removed in the next version of Ember Data.'); |
There was a problem hiding this comment.
I think we want to change this deprecation message to the one in _findById.
|
I've made the changes. I've included a deprecation warning for Also, I've left |
|
Hi @gevious. Thanks for doing all the work on this pr. Unfortunately, I'm going to close it because since Ember Data 2.0 is released, I think we missed the opportunity to make this change in a semver compatible way. Feel free to re-open if you disagree. |
Closes #3250