-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feat: Add experimental CLI for LoopBack 4 #693
Conversation
2f25718
to
4043481
Compare
a067c08
to
ceb1764
Compare
packages/cli/README.md
Outdated
--skip-cache # Do not remember prompt answers Default: false | ||
--skip-install # Do not automatically install dependencies Default: false | ||
--applicationName # Application name | ||
--description # Description for theapplication |
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 application
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.
Good catch. Fixed.
|
||
it('creates files', () => { | ||
assert.noFile(['tslint.json', 'tslint.build.json']); | ||
assert.jsonFileContent('package.json', props); |
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 wonder how assert.jsonFileContent
works. Below we are checking for prettier
here, but not in this particular test?
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.
See https://github.com/yeoman/yeoman-assert#api. The assertions here focus on verifying the omission of tslint related artifacts.
Trial run result (for app):
{"greeting":"Hello from LoopBack","date":"2017-11-02T23:19:24.416Z","url":"/ping","headers":{"host":"localhost:3000","connection":"keep-alive","user-agent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.100 Safari/537.36","upgrade-insecure-requests":"1","accept":"text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8","accept-encoding":"gzip, deflate, br","accept-language":"en-US,en;q=0.8"}} |
greeting: 'Hello from LoopBack', | ||
date: new Date(), | ||
url: this.req.url, | ||
headers: Object.assign({}, this.req.headers), |
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 found including the headers to be a bit ugly for UX, but feel free to keep 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.
Haha, I have a json viewer extension installed for my browser and it looks fine :-). Surprisingly, most browsers don't render json response nicely by default.
@@ -0,0 +1,3 @@ | |||
# Repositories |
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 Mixin
, Providers
, etc.. pages have a great deal of description. Can we add some links to our docs or brief explanations here as well?
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 READMEs are copied from https://github.com/strongloop/loopback4-extension-starter.
@virkt25 Do you make the docs for Mixin/Provider/Decorator available to http://loopback.io/doc/en/lb4/Concepts.html?
We should only keep links in the README.md to be scaffolded into user projects.
@@ -0,0 +1,3 @@ | |||
# Controllers |
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 Mixin
, Providers
, etc.. pages have a great deal of description. Can we add some links to our docs or brief explanations here as well?
@@ -0,0 +1,3 @@ | |||
# Repositories |
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 Mixin
, Providers
, etc.. pages have a great deal of description. Can we add some links to our docs or brief explanations here as well?
@@ -0,0 +1 @@ | |||
# Acceptance tests |
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 Mixin
, Providers
, etc.. pages have a great deal of description. Can we add some links to our docs or brief explanations here as well?
@@ -0,0 +1 @@ | |||
# Integration tests |
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 Mixin
, Providers
, etc.. pages have a great deal of description. Can we add some links to our docs or brief explanations here as well?
@@ -0,0 +1 @@ | |||
# Unit tests |
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 Mixin
, Providers
, etc.. pages have a great deal of description. Can we add some links to our docs or brief explanations here as well?
This is expected as the browser will send |
Considering our past experience with yeoman, I think we should not be basing any new tooling on yeoman. We should use inquirer and ejs templating directly. Having said that, I am happy to use yeoman for this initial experimental implementation, as it makes it easier for us to get something up and running 👍 I don't have bandwidth to review this pull request in details, I'll leave that up to @strongloop/sq-lb-apex |
Usage: npm i -g @loopback/cli lb4 app <name> lb4 extension <name>
Do we have a list of pain points with Yeoman somewhere? The latest version seems to be working out pretty well. I was able to register generators with yeoman-environment explicitly without the magic and the module name doesn't have to be @eddiemonge As a maintainer of Yeoman, do you have any insights? |
I'm not well-versed enough in templating and generators to have an informed opinion. I'd probably have to take a few days to play around with it and see what I think. If we're concerned about blocking this PR for that long, then I won't stand in the way. |
@kjdelisle I would like to have it merged as soon as possible so that we can start to use it to scaffold new extensions/apps. Maybe we can move forward as follows:
|
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 recap, it looks good to me, but I admit that I don't really know what would be best.
When can we expect this experimental cli to be published? |
@jonathan-casarrubias It's published under |
Ok cool, installing.... |
Description
Usage:
For reviewers, you can check out the branch and run
node packages/cli/bin/cli app <my-app>
ornode packages/cli/bin/cli extension <my-ext>
.Related issues
Checklist