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

support for relative paths on sequelize.import #901

Merged
merged 2 commits into from Sep 20, 2013

Conversation

6 participants
@accerqueira
Contributor

accerqueira commented Sep 12, 2013

Thanks for all the work on sequelize!

This patch makes this possible:

var Project = this.sequelize.import("assets/project")
or
var Project = this.sequelize.import("./assets/project")
where only
var Project = this.sequelize.import(__dirname + "/assets/project")
would work, previously (and still does).

I hope this contribution is usefull

@ricardograca

This comment has been minimized.

Contributor

ricardograca commented Sep 12, 2013

Can you update the description to provide an example on how to use it?

@durango

This comment has been minimized.

Member

durango commented Sep 12, 2013

Nice work! :) Any chance we can get a test for a file that doesn't exist / errors? :)

@durango

This comment has been minimized.

Member

durango commented Sep 12, 2013

@ricardograca its pretty self explanatory O_o its relative paths for importing instead of having to define an absolute path

@ricardograca

This comment has been minimized.

Contributor

ricardograca commented Sep 12, 2013

I know that and you know that, but some future user reading this pull-request may not know that :) But, yes, it's probably obvious. The docs will need updating though.

@accerqueira

This comment has been minimized.

Contributor

accerqueira commented Sep 12, 2013

Thanks for the feedback! Updated the description with examples.
I looked for docs to update, but couldn't find... can you point me in the right direction?

@selfcontained

This comment has been minimized.

Contributor

selfcontained commented Sep 12, 2013

docs are in a separate repo - https://github.com/sequelize/sequelize-doc

if (url.parse(path).pathname.indexOf('/') !== 0) {
// make path relative to the caller
var callerFilename = Utils.stack()[1].getFileName();
path = url.resolve(callerFilename, path);

This comment has been minimized.

@janmeier

janmeier Sep 13, 2013

Member

Wouldn't it be more proper to use path.resolve for this?

This comment has been minimized.

@accerqueira

accerqueira Sep 13, 2013

Contributor

Yes, sorry about that.
What variable name should "path" module be assigned to? Is "Path" ok?

This comment has been minimized.

@janmeier

janmeier Sep 13, 2013

Member

Ok for me :-)

@sdepold

This comment has been minimized.

Member

sdepold commented Sep 20, 2013

cool :)

@sdepold

This comment has been minimized.

Member

sdepold commented Sep 20, 2013

tests aren't running for me

Cannot find module '/Users/sdepold/Projects/sequelize/test/sequelize.test.js/assets/project'

@sdepold sdepold merged commit fa069ec into sequelize:master Sep 20, 2013

1 check failed

default The Travis CI build failed
Details

@accerqueira accerqueira deleted the accerqueira:feature/import-relpath branch Sep 20, 2013

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