-
Notifications
You must be signed in to change notification settings - Fork 1.1k
docs(example-getting-started): split tutorial into separate files #1001
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
2e5ab1b to
4640e6c
Compare
raymondfeng
left a comment
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.
Great work to make the guide progressive!
Please address my comment before merging.
|
|
||
| Now that your app is ready to go, try it out with your favourite REST client! | ||
| Start the app (`npm start`) and then make some REST requests: | ||
| - `POST /todo` with a body of `{ "title": "get the milk" }` |
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 probably should illustrate the usage of API Explorer and/or curl here.
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 don't intend to improve the individual documents at this time. There's quite a bit of work required across all of the pieces to make it a more cohesive and instructive tutorial, but I want to make sure that the wireup between this repository and loopback.io is finished first.
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.
That's fair. I'm fine with follow-up PRs.
4640e6c to
1ab8100
Compare
| @@ -0,0 +1,30 @@ | |||
| ## Prerequisites | |||
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.
Can we signify that if they are to follow the instructions here, they get the fully functional app, and if they follow the other steps, they're following a tutorial to build 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.
See #1001 (comment)
virkt25
left a comment
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.
Haven't reviewed the content of the files. Not sure if I'm reviewing content or there will be follow up PRs for that as per your comment to Raymond.
| glob('**', { | ||
| cwd: path.join(__dirname, `../../example-${VALID_EXAMPLE}`), | ||
| ignore: '@(node_modules|dist*|api-docs)/**', | ||
| ignore: '@(node_modules|dist*|api-docs|docs)/**', |
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 think this is no longer needed ... right?
Break up the pieces of the tutorial so that they can be served as individual links on LoopBack.io under the LB4 docs. re #729
1ab8100 to
70fba54
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.
I started review on the documents, but then I read your comments on intending to improve the contents of the documents later on, so just ignore my comments (or have it in the back of your mind for when you do change up the contents inside).
| ## Prerequisites | ||
|
|
||
| Before we can begin, you'll need to make sure you have some things installed: | ||
| - [Node.js](https://nodejs.org/en/) at v6.x or greater |
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.
8.x
| @@ -0,0 +1,13 @@ | |||
| ### Create your app scaffolding | |||
|
|
|||
| Install the `@loopback/cli` package. This will give you the command-line | |||
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 has been covered in the prerequisite page. This line should be phrased so that it 'remind' the users to have it installed instead
| toolkit that can generate a basic REST app for you. | ||
| `npm i -g @loopback/cli` | ||
|
|
||
| Next, navigate to whichever directory you'd like to create your new project |
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'm already confused here. The previous page tells me to download example-getting-started but we don't do anything with it? Maybe we should clarify somewhere that example-getting-started has only been downloaded for users' reference
| import {ApplicationConfig} from '@loopback/core'; | ||
| import {RestApplication} from '@loopback/rest'; | ||
| import {PingController} from './controllers/ping-controller'; | ||
| import {Class, Repository, RepositoryMixin} from '@loopback/repository'; |
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 don't use Class or Repository yet
| @@ -0,0 +1,104 @@ | |||
| ### Building the Todo model | |||
|
|
|||
| The Todo model will be the object we use both as a Data Transfer Object (DTO) on | |||
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.
A link to what a Data Transfer Object might be helpful, unless we assume users know what this is (I don't 😆 ).
| // elsewhere... | ||
|
|
||
| // with index.ts | ||
| import {Foo, Bar, Baz} from './models'; |
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.
Isn't there the danger of creating circular dependencies here? The examples use models, but what if they were something with complicated dependencies? I remember having to go and change the imports around so that they were imported straight from their original files instead of from index.ts to solve some problems I was having.
| - a unique id | ||
| - a title | ||
| - a description that details what the todo is all about | ||
| - a boolean flag for whether or not we've completed the task. |
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.
consistent periods or not at all please
| - a description that details what the todo is all about | ||
| - a boolean flag for whether or not we've completed the task. | ||
|
|
||
| For the Legacy Juggler to understand how to work with our model class, 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.
Not a grammar master, but we might need a comma here: Legacy Juggler, to. Or we might need to change the phrasing so that we don't have too many commas in this sentence.
|
Travis is still all gummed up on macOS right now, so going to go ahead and land this. |
|
@kjdelisle the new README looks pretty useless to me: Please make sure to beef it up in your follow-up pull requests, so that people consuming this content from GitHub and/or At minimum, I would expect a TOC-like list pointing to individual files ("chapters"). |

Break up the pieces of the tutorial so that they can be served as individual links on LoopBack.io
under the LB4 docs.
re #729
Checklist
npm testpasses on your machinepackages/cliwere updatedpackages/example-*were updated