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

Add Sequelize#model to fetch DAOFactory of previously defined model #868

Merged
merged 1 commit into from Sep 1, 2013

Conversation

3 participants
@jwilm
Contributor

jwilm commented Aug 30, 2013

This provides some basic currying to the user for fetching DAOFactorys. I would update the docs too if I could figure out how they're generated.

@durango

View changes

lib/sequelize.js Outdated
@returns {DAOFactory} The DAOFactory for daoName
*/
Sequelize.prototype.model = function(daoName) {
if(!this.isDefined(daoName))

This comment has been minimized.

@durango

durango Aug 30, 2013

Member

Curly braces please :)

@durango

View changes

test/sequelize.test.js Outdated
var self = this
expect(function() {
self.sequelize.model('Project')
}.bind(this)).to.throw(/project has not been defined/i)

This comment has been minimized.

@durango

durango Aug 30, 2013

Member

Is the bind necessary?

@jwilm

This comment has been minimized.

Contributor

jwilm commented Aug 30, 2013

Oh shoot, not at all. I had the bind before I looked around and noticed the repository typically defines self = this. Will push an update shortly...

@durango

This comment has been minimized.

Member

durango commented Aug 30, 2013

No worries man, sorry for being nitpicky :( other than that very cool :D and thanks!

@jwilm

This comment has been minimized.

Contributor

jwilm commented Aug 30, 2013

No problem! I understand wanting consistent code style within a project. Incidentally, that is how I accidentally ended up with that self = this and .bind(this) for the same function call.

The updates you requested are incorporated - not sure what sort of notification you get when I amend to the commit of a pull request.

@sdepold

This comment has been minimized.

Member

sdepold commented Sep 1, 2013

@jwilm we mostly skip the .bind calls because if performance. its sad, but defining a variable which reflects this is still way faster than .binding a function :(

@sdepold

This comment has been minimized.

Member

sdepold commented Sep 1, 2013

and thanks for the PR :)

sdepold added a commit that referenced this pull request Sep 1, 2013

Merge pull request #868 from jwilm/model-accessor
Add Sequelize#model to fetch DAOFactory of previously defined model

@sdepold sdepold merged commit a1adbad into sequelize:master Sep 1, 2013

1 check passed

default The Travis CI build passed
Details

@jwilm jwilm deleted the jwilm:model-accessor branch Sep 1, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment