Skip to content

Conversation

@mc0
Copy link
Owner

@mc0 mc0 commented Feb 25, 2015

Sorry this is lumped together but there was a ton of stuff I wanted to change. The goal of this version bump is to both correct some inconsistencies and also clean up some code that seems neglected.

We inherited a bunch of functions that were iterating over objects unsafely and there were a few spots using arguments passed in (which v8 won't optimize).

I also added a ton of documentation (but still incomplete). I think we can write more as time moves along but it's better than the nonexistent docs we have now.

I got rid of the coffeescript dev dependency and removed the model.coffee since it was needlessly duplicating tests from model.js.

bedrock.js Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

instead of exports which is already a variable in Node.js can this just be called Bedrock or something stating that its actually the bedrock object were going to be putting stuff one?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe exportObj idk

@jameshartig
Copy link
Collaborator

In response to all the micro-optimizations you're making to looping through arrays I don't think its worth it. See: jashkenas/backbone#3497
Plus you're changing the behavior in a few spots when you're looping backwards.

James Hartig and others added 3 commits May 11, 2015 23:39
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.

3 participants