-
Notifications
You must be signed in to change notification settings - Fork 182
Allow explicit numeric datatype #254
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
Conversation
496ceae
to
7014197
Compare
7014197
to
7ebfb98
Compare
lib/migration.js
Outdated
return colDefault ? (' DEFAULT ' + colDefault) : ''; | ||
}; | ||
|
||
// override this function from base connector to allow postgres |
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.
to allow postgres connector to
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.
Fixed.
test/postgresql.test.js
Outdated
|
||
it('should run migration', function(done) { | ||
db.automigrate('PostWithBoolean', function() { | ||
db.automigrate(['PostWithBoolean', 'Expense'], function() { |
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.
what does PostWithBoolean
do?
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.
PostWithBoolean
is another model attached to the same datasource. It was already there before for other tests. I only added the Expense
model.
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.
oops, yeah!
8fc50a8
to
60fb48a
Compare
lib/migration.js
Outdated
return null; | ||
} | ||
var colLength = columnMetadata && columnMetadata.dataLength || | ||
prop.length || prop.limit; |
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.
This should be indented one level?
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.
Fixed.
60fb48a
to
9fe7aaa
Compare
lib/migration.js
Outdated
// info on setting column specific properties | ||
// i.e dataLength, dataPrecision, dataScale | ||
// https://loopback.io/doc/en/lb3/Model-definition-JSON-file.html | ||
if (colType && colLength) |
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.
I would refactor this since colType
is a common condition:
if(colType) {
if (colLength) {
// return ..
}
if ( colPrecision && colScale) {
// return ...
}
}
ba5e80a
to
a463388
Compare
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.
LGTM
* Add docker setup (#256) (Sakib Hasan) * Allow explicit numeric datatype (#254) (Sakib Hasan) * Allow non-id serial properties (#198) (zbarbuto) * Revert PR #246 (#248) (Sakib Hasan) * Add loopback-connector as peer dependencies (#246) (Russ Tyndall) * Fix operations on ended transactions (zbarbuto) * dbdefaults: Cleanup InvalidDefault def after test (Kevin Delisle) * Reuse the data source to avoid too many clients (Raymond Feng)
Description
connect to #250
Related issues
Checklist
guide