-
-
Notifications
You must be signed in to change notification settings - Fork 525
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
Model association for sequalize-cli 4.0.0 updates #620
Model association for sequalize-cli 4.0.0 updates #620
Conversation
@sushantdhiman I think the tests will pass if they can just be restarted. Looks like one got hung up for a very long time. Are you able to restart the build? I don't see a button to do it so I don't think I have permissions. If not I can try to find something to push to kick off another build. |
No problem, I will retry that test |
@@ -197,7 +197,7 @@ const _ = require('lodash'); | |||
|
|||
const targetContent = attrUnd.underscored ? | |||
'underscored: true' | |||
: '{\n classMethods'; | |||
: '{});'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert associate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for being a bit dense. I am not sure what you are wanting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to check if generated model contain associate
method, just scanning file with .associate
around here should be enough
Just add .pipe(helpers.ensureContent('.associate'))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. Will do.
test/model/create.test.js
Outdated
@@ -210,7 +210,8 @@ const _ = require('lodash'); | |||
.src(Support.resolveSupportPath('tmp', 'models')) | |||
.pipe(helpers.readFile('user.js')) | |||
.pipe(helpers.ensureContent(targetContent)) | |||
.pipe(helpers.teardown(done)); | |||
.pipe(helpers.teardown(done)) | |||
.pipe(helpers.ensureContent('.associate')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.pipe(helpers.ensureContent('.associate'))
, Should come before teardown
, I think tests will fail
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hah. okay, they passed locally. Will move it. Totally didn't realize those where teardowns.
26b6608
to
ba94a1c
Compare
@sushantdhiman okay. I moved the line up above teardown and force pushed a rebase to keep the commit history a bit cleaner. |
@sushantdhiman looks like we are good to go. It is now mergeable and the tests have all passed. |
Created this to deal with a merge conflict so it can be merged. Preserved commits for contribution credit.