-
Notifications
You must be signed in to change notification settings - Fork 365
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
Datasource documentation tune-up #1333
Conversation
1fcf7d2
to
3197131
Compare
6ef1988
to
9f8648c
Compare
@raymondfeng When you're feeling better, please take a look at the comments marked Once this portion is complete, I'll move onto tuning up the corresponding juggler documentation in loopback.io |
9f8648c
to
199a9a4
Compare
lib/datasource.js
Outdated
* @param callback | ||
* @returns {Promise} | ||
* @emits connect |
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.
The event name is connected
.
lib/datasource.js
Outdated
* Define relations for the model class from the relations object | ||
* @param modelClass | ||
* @param relations | ||
/** |
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.
IIRC, we intentionally use /*!
to not expose this method to generated API references.
lib/datasource.js
Outdated
@@ -731,23 +745,35 @@ DataSource.prototype.mixin = function(ModelCtor) { | |||
}; | |||
|
|||
/** | |||
* See ModelBuilder.getModel | |||
* | |||
* // XXX(kev): Deprecation notice for 4.x? |
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.
Do we want to use a generic tag, such as // REVIEW
or // FIXME
?
199a9a4
to
4ba4752
Compare
fcc7257
to
47497f3
Compare
lib/datasource.js
Outdated
@@ -949,7 +990,7 @@ DataSource.prototype.autoupdate = function(models, cb) { | |||
* @property {Boolean} views If true, include views; if false, only tables. | |||
* @property {Number} limit Page size | |||
* @property {Number} offset Starting index | |||
* | |||
* @returns {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.
We should use ModelDefinition[]
to differentiate from Model
which is for the mode class.
lib/datasource.js
Outdated
* The synchronous version of discoverModelDefinitions. | ||
* @options {Object} options The options | ||
* @property {Boolean} all If true, discover all models; if false, discover only models owned by the current user. | ||
* @property {Boolean} views If true, nclude views; if false, only tables. | ||
* @property {Number} limit Page size | ||
* @property {Number} offset Starting index | ||
* @returns {*} | ||
* @returns {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.
ModelDefinition[]
@loay Don't forget to squash and rename the commits to remove "wip" |
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
connected to https://github.com/strongloop-internal/scrum-apex/issues/175