Use app.restApiRoot #10
Conversation
Use `app.docs()` to expose Swagger specs. This way we don't have to depend on loopback's dependency strong-remoting.
|
Test PASSed. To trigger a build add comment - ".test\W+please" |
| options = extend({}, options); | ||
| options.basePath = options.basePath || loopbackApplication.get('restApiRoot'); | ||
|
|
||
| loopbackApplication.docs(options); |
There was a problem hiding this comment.
I'd like to deprecate app.docs in the core application in favor of using swagger directly here.
There was a problem hiding this comment.
Why is it better to access swagger directly? Things I don't like about that approach: The consumer has to know intimate details about how loopback is composed to be able to find strong-remoting and the swagger extension (requireLoopbackDependency('strong-remoting/ext/swagger') is a serious code smell). It also has to know that loopback is using strong-remoting and the swagger builder requires a list of remotable methods (as opposed to e.g. list of shared classes). If we later decided to extend swagger descriptors with e.g. model definitions and add another parameter to swagger() function, then we will break compatibility of new strong-remoting/loopback with the old loopback-explorer. It wouldn't happen with app.docs() approach, because it's abstracting away all these details.
If we want to deprecate app.docs then I would prefer to either deprecate strong-remoting/ext/swagger too and move the code to this project, or expose strong-remoting/ext/swagger in loopback exports. E.g. loopback.remoting.ext.swagger, where loopback.remoting is require('strong-remoting') and the rest is filled by strong-remoting.
There was a problem hiding this comment.
We should deprecate strong-remoting/ext/swagger and move that code into this project. That sounds like a project on its own, so depending on app.docs for this change is reasonable.
|
I have created #11 to track this idea. Is there anything else to improve? May I merge? |
|
LGTM |
|
Test FAILed. To trigger a build add comment - ".test\W+please" |
|
test please |
|
Test FAILed. To trigger a build add comment - ".test\W+please" |
|
The build says it has installed loopback from master (which is at v1.5.0 at the moment) but the installed version is v1.4.2: /cc @rmg |
|
test please |
|
Test PASSed. To trigger a build add comment - ".test\W+please" |
|
The build is passing now. A possible explanation is that loopback v1.5.0 was not yet published to our artifactory at the time when the previous builds were running. |
|
@bajtos that seems likely. Another cause would be if |
This pull request contains commits from #9 relevant to the new config option
restApiRoot./to: @ritch please review.