-
Notifications
You must be signed in to change notification settings - Fork 386
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
types: add DateString type #373
Conversation
The definition this documentation currently refers to hasn't been written yet, please hold off on merging this until I post an update. |
@crandmck Actually, Rand, could I get you to review the JSDoc portion of this PR? |
@@ -65,6 +65,17 @@ The following table summarizes LoopBack's data types. | |||
</td> | |||
</tr> | |||
<tr> | |||
<td>DateString</td> | |||
<td> | |||
<p>LoopBack<a href="http://apidocs.strongloop.com/loopback-datasource-juggler/#datestring" class="external-link" rel="nofollow"> DateString</a> object</p> |
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.
will we have an anchor for #datestring in the apidoc?
@@ -65,6 +65,17 @@ The following table summarizes LoopBack's data types. | |||
</td> | |||
</tr> | |||
<tr> | |||
<td>DateString</td> | |||
<td> | |||
<p>LoopBack<a href="http://apidocs.strongloop.com/loopback-datasource-juggler/#datestring" class="external-link" rel="nofollow"> DateString</a> object</p> |
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.
There is no tag with id datestring
on that page. Other than that LGTM
@@ -65,6 +65,17 @@ The following table summarizes LoopBack's data types. | |||
</td> | |||
</tr> | |||
<tr> | |||
<td>DateString</td> | |||
<td> | |||
<p>LoopBack<a href="http://apidocs.strongloop.com/loopback-datasource-juggler/#datestring" class="external-link" rel="nofollow"> DateString</a> object</p> |
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.
Right now, this link doesn't exist. I presume there is a PR somewhere for it?
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 assume that's what @kjdelisle is talking about re loopbackio/loopback-datasource-juggler#1356. Once that lands (with some modifications) and juggler is published to npm, then the link should work.
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
Looks fine, but as noted above, we should not land this until the change to juggler is published to npm, since that's when the feature will become broadly available and the links to the API docs will work as well. |
connected to loopbackio/loopback-connector-mysql#149