Conversation
The version that bower uses enabled fakeTimers in sinon which we don't want.
|
Looks like this isn't quite ready yet. I'm running NestedModel against the backbone tests directly and it still has some problems. |
…ed from Backbone.Model's set
|
It's now running backbone-nested against backbone's own model tests. All the tests passed locally. I would think that it means it works with backbone 1.x now. Probably would need a bit more testing to make sure things work. |
test/index.html
Outdated
There was a problem hiding this comment.
Could you include some reasoning here about why we are running the tests for non-nested models? I think I understand the reasoning, but want to a) make sure we're on the same page, and b) clarify for others who might be browsing the code.
There was a problem hiding this comment.
The idea is to run the tests against Backbone's Model to verify these tests which test the the usage of a model in the same scenarios as backbone itself will be valid.
If we want the basic cases to work the same way as Backbone.Model, then the tests should succeed when using Backbone.Model and not just Backbone.NestedModel.
If the model.js tests fail for either Backbone.Model or Backbone.NestedModel, that means that in the basic usage, the two are not compatible.
There was a problem hiding this comment.
Fair enough. Mind adding a concise comment about that in here?
|
Nice work, much appreciated! Please remove all the commented-out code that isn't needed anymore, but other than my comments above, looks very very close! |
|
How's it going with the changes? I'm eagerly waiting for this merge to happen. |
|
Sorry, been busy with other work. I'll try to get this finished this week. |
|
Let me know if you need a hand. |
|
Any testing you could give would be great. |
Update backbone dep to ~1.1.0 Move jslitmus to be a devDep.
This reverts commit 14bb6e3.
|
Mind merging |
|
Git is telling me it's all update to date. It's weird that it's still displaying those commits in this PR even though #95 was merged into master already. |
|
Sure you're merging from my master? git pull git@github.com:afeld/backbone-nested.git master |
|
You should see 9e3f9d2 in your history. |
|
Oh, yeah, might be the wrong master, heh. Let me try again. |
|
I think that worked. |
backbone-nested.js
Outdated
There was a problem hiding this comment.
Possibly not. However, since we are passing it to a validation function provided by the user, we probably make sure that if they mutate these attrs, it doesn't mutate the internal model state.
There was a problem hiding this comment.
Maybe add a line note for it, in that case?
bower.json
Outdated
There was a problem hiding this comment.
The file available from bower has useFakeTimers set to true which is causing problems.
Backbone 1.0 tweaks
|
Many many 👏 on this! |
|
Also, welcome @gkatsev as a collaborator on the repo! I'll still do everything through PRs, and ask that everyone else do the same. |
|
Sweet. |
Load up backbone 1.x and make sure that all the tests pass against it. This addresses #72.
This includes the commits from the previous PR #95.