-
-
If these are instance variables, you don't want them here. They should be set in initialize(). Every object should specifically set every instance variable it will use in its lifetime in a default value in initialize(). Any object in JavaScript that dynamically defines new variables on itself post-initialize takes a performance and memory hit, since the VM can't share as much state. (There was a good writeup on this somewhere, but I can't find it off-hand.)
-
-
Probably, we should just use options.blah instead of setting these values. Then you'd define: collection = new RB.RepositoryCommits([], { urlBase: url, start: start }); The collection would then use this.options.urlBase/start instead of this.urlBase/start, which is more Backbone-ish. -
-
-
Every Model has an id field, so we don't need this. By setting id below, the model's ID will be the provided ID. I think that's okay, but worth noting.
-
-
-
I feel like this should be a Resource model. I don't know that it's really worth having inconsistent modelling for these things. I also want to start putting everything api-related in resources/{collections,models}/, so I think this and the other resource models/collections should go in there.
Add javascript models for Repository, Branch, and Commit.
Review Request #4266 — Created June 26, 2013 and submitted
Add javascript models for Repository, Branch, and Commit. This change adds models and collections for repositories, branches, and commits, along with basic tests for the fetching and parsing logic. This implements the client side of the new commits and branches resources.
Ran unit tests. Used these models in my larger post-commit changes.
| Description | From | Last Updated |
|---|---|---|
|
If these are instance variables, you don't want them here. They should be set in initialize(). Every object should specifically … |
 chipx86
|
|
|
This is just testing that we set it above. Doesn't seem to be very useful as-is. |
chipx86
|
|
|
Probably, we should just use options.blah instead of setting these values. Then you'd define: collection = new RB.RepositoryCommits([], { urlBase: … |
chipx86
|
|
|
Did you mean to leave this? |
chipx86
|
|
|
Can you wrap this? |
chipx86
|
|
|
Every Model has an id field, so we don't need this. By setting id below, the model's ID will be … |
chipx86
|
|
|
Missing ; |
chipx86
|
|
|
I feel like this should be a Resource model. I don't know that it's really worth having inconsistent modelling for … |
chipx86
|
- Diff:
-
Revision 2 (+291)
- Diff:
-
Revision 3 (+300)
- Change Summary:
-
Leftover debug output.
- Diff:
-
Revision 4 (+299)