-
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
docs(example-getting-started): tutorial cleanup #1056
Conversation
[next section here](2-scaffold-app.md). | ||
|
||
If you'd like to see the final results of this tutorial as an example | ||
application, then follow these steps: |
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, then
is not needed for English.
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 except a nitpick.
@@ -1,4 +1,4 @@ | |||
## Prerequisites | |||
## Setup | |||
|
|||
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.
v8.x or greater
If you'd like to see the final results of this tutorial as an example | ||
application, then follow these steps: | ||
|
||
1. Run the `lb4 example` command to clone the getting-started 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.
I'd say Run the 'lb4 example' command to select and clone the getting-started repository
to be clear
@@ -1,15 +1,22 @@ | |||
### Create your repository | |||
|
|||
The repository pattern is one of the more fundamental differences between | |||
LoopBack 3 and 4. In 3, you would use the model class definitions themselves |
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'd say In LB3
or In v3
instead of just 3 here. Feel free to ignore though.
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 quickly skimmed through the new content, this is great!
I think we should eventually stop calling the Juggler Bridge as "legacy", considering that we decided the juggler is going to stay with us (#537). This is may be off topic here, but it caught my attention that quite few places in the newly added text are using the term Legacy Juggler Bridge.
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.
Looking back at my comments, I think I've been way too picky.
After having gone through these pages, I feel like it'd be better to not have a lead-in sentence to the next section at all. To me, the lead-ins don't tell me enough about what the next section is and what it's for, and adding in descriptions for that would take away from the page it's leading out of. I think it's sufficient enough to tie in the adjacent sections at the beginning of each pages.
2. Download the "getting-started" application. | ||
|
||
If you'd like to continue the tutorial, then jump to the | ||
[next section here](2-scaffold-app.md). |
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.
Instead of saying next section here
, I'd rather see something like scaffolding your application section
|
||
<!-- TODO: Add screenshot of terminal here to illustrate what we mean. --> | ||
|
||
For this tutorial, when prompted with the options for selecting things like |
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'd like to see here a brief description of the roles of each artifacts being generated.
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.
Currently, those artifacts haven't been completed. I would prefer to leave it non-specific for now, or add a link in the future to an "artifacts" section that gives them more details.
For the purpose of this tutorial, telling people about all of the artifacts that they'll use in a typical application is a natural result of following along.
whether or not to enable certain project features (loopback's build, tslint, | ||
mocha, etc.), leave them all enabled. | ||
|
||
When you're ready, we'll begin laying the groundwork for our application by |
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.
What is this groundwork that we're laying out? I think a little explanation of what setting up the juggler would do would be helpful
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.
On second look, I noticed that this is covered in the next section, but I still think it'd be helpful for the readers to provide them with what to expect and a reason to why we're doing this as the next step.
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 my over-arching review comment above.
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 tried explaining it but it comes across as confusing and distracting; there's no way to easily explain why you're doing all these things up front without drastically increasing the difficulty of the tutorial.
|
||
It also provides many of the functions and interfaces we'll require for setting | ||
up our new LoopBack application, which is why we're starting here. | ||
|
||
Jump into the directory for your new application. You'll see a folder structure | ||
similar to this: | ||
``` |
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 folder structure should be duplicated in the page before.
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.
@shimks Could you be more specific as to what you're expecting to see?
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 changed my mind. I originally wanted descriptions for the artifacts in the previous page and thought the folder structure diagram would go nicely with it, but since we're not doing that this is fine as it is I think.
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 @shimks meant that the previous step should have a table explaining the generated app for the user.
|File/Folder|Description|
| dist/|Folder containing the compiled application code (TS -> JS)|
|README.md|Description and instructions for your Application|
I think that would be a good idea and it would eliminate the need for the file structure on this page.
@@ -12,7 +21,8 @@ create two files: | |||
The `index.ts` file is an export helper file; this pattern is a huge time-saver | |||
as the number of models in your project grows, because it allows you to point | |||
to the _directory_ when attempting to import types from a file within the target | |||
folder. We will use this concept throughout the tutorial! | |||
folder. **We will use this concept throughout the tutorial! For more info, | |||
see TypeScript's [Module Resolution](https://www.typescriptlang.org/docs/handbook/module-resolution.html) docs.** | |||
```ts |
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.
Is it possible to quote a code block 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.
I avoid doing it because it seems like different Markdown parsers will interpret this in different ways, either deciding to respect the indent, or ignore it completely.
model type we're working with, as well as its ID type. We'll also inject our | ||
datasource so that this repository can connect to it when executing data | ||
operations. | ||
`DefaultCrudRepository` class from |
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.
Maybe it could be inferred from the name of the class, but I think we should mention that DefualtCrudRepository
provides the CRUD operations for you.
@@ -5,7 +5,7 @@ Now, we'll create a controller to handle our Todo routes. Create the | |||
- `index.ts` (export helper) | |||
- `todo.controller.ts` | |||
|
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.
Quick explanation of what a controller's role is and a link to it's docs page would be good here.
@@ -64,6 +64,21 @@ export class TodoController { | |||
} |
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 code block would've been too much for me to take in if I was a new user. We should move the code block at the end of the docs and replace it with just the constructor and a single route so that it's easier to take in.
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 agree; there are plenty of code examples out there that aren't particularly brief, but are still useful. This example has repetition of common logic with simple differentiators to make it clear how to handle all of the basic verbs used for a REST API.
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.
Additionally, it has an example under the POST request showing how you can add business logic (even though it's simple logic).
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.
Actually, thinking about it more, you're right that this is too much to dump on at once.
I guess I interpreted the statement to mean that we shouldn't have the final product of the controller in one code snippet at all. Realistically, though, we could spend more time building up to that point, since we're probably not doing enough to explain what's happening step-by-step.
as adding in our new controller binding. | ||
We'll define a new helper function for setting up the repositories, and | ||
LoopBack's [boot module](https://github.com/strongloop/loopback-next/tree/master/packages/boot) will | ||
take care of the other artifacts for us! |
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.
How will it take care of the other artifacts? Why do these artifacts even need to be taken care of in the first place? By what method? What is a binding
?
@@ -29,17 +39,20 @@ import {Bar} from './models/bar.model'; | |||
import {Baz} from './models/baz.model'; | |||
``` | |||
|
|||
In our Todo model, we'll create a basic representation of what would go in | |||
a Todo list. Our model will include: | |||
For our Todo model to represent our Todo instances, it will need: |
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 before we get into what our Todo
model would look like, we should describe at a top-level what the function of Todo
is; Something like this:
We want an entry of Todo to contain:
- a name of the task
- a description of the task
- a checkmark for its completion status
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 before we get into what our Todo
model would look like, we should describe at a top-level what the function of Todo
is; Something like this:
We want an entry of Todo to contain:
- a name of the task
- a description of the task
- a checkmark for its completion status
@bajtos I thought that we had the Juggler Bridge as a part of v3, and that v4's Juggler would technically be a "rewrite" in the sense that we would be creating a new monorepo for the repository interfaces, validation logic, relation logic and possibly the connectors? |
020e9b8
to
382ad6b
Compare
@shimks Some of your comments were left on lines of code that haven't changed as a result of my subsequent commits. PT(another)L and make sure that I've addressed them as you expect. :) |
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.
Some of my comments were left unaddressed, but apart from that LGTM 🚢
382ad6b
to
9ed2e13
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.
Pointed out a couple of comments that may have been missed.
Our Todo instances will need to have a Datasource to go with them if we're going | ||
to create a useful application, so now we'll construct a datasource instance | ||
that we can use in conjunction with our Todo model to perform CRUD operations | ||
against a database. | ||
|
||
Create a new folder in the root directory of the project called `config`, | ||
and then inside that folder, create a `datasources.json` file. For now, we'll |
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.
Instead of For now,
I think we should go with For the purpose of this tutorial,
since we don't ever change our datasource within this tutorial
@@ -33,6 +35,9 @@ export const db = new DataSourceConstructor(config); | |||
This will give us a strongly-typed datasource export that we can work with to |
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.
In line 20, could we get a small explanation of why we're creating db.datasource.ts
? I think an explanation of its context to the config file would go a long way.
@@ -1,7 +1,9 @@ | |||
### Building a Datasource | |||
|
|||
Before we can begin constructing controllers and repositories for our | |||
application, we need to define our datasource. | |||
Our Todo instances will need to have a Datasource to go with them if we're going |
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 me, this isn't clear enough as to what a datasource is. It doesn't need to be verbose, but I think an explanation of what a datasource is and what its role is in a LoopBack application would be helpful here.
@@ -1,15 +1,22 @@ | |||
### Create your 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.
I think this header should move down to line 9 and be replaced with just Repository
. Reason being that the paragraph beneath goes into a difference between lb4 and lb3 which has nothing to do with creating the repository itself.
@@ -29,17 +39,20 @@ import {Bar} from './models/bar.model'; | |||
import {Baz} from './models/baz.model'; | |||
``` | |||
|
|||
In our Todo model, we'll create a basic representation of what would go in | |||
a Todo list. Our model will include: | |||
For our Todo model to represent our Todo instances, it will need: |
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 before we get into what our Todo
model would look like, we should describe at a top-level what the function of Todo
is; Something like this:
We want an entry of Todo to contain:
- a name of the task
- a description of the task
- a checkmark for its completion status
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 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.
Aside from minor nitpicks, LGTM. Make sure to wait for @virkt25 's review. I'm sure he has a lot in mind to make our tutorial amazing.
export class TodoController { | ||
constructor( | ||
@repository(TodoRepository.name) protected todoRepo: TodoRepository, | ||
) {} |
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 line break here would be great, but that change probably also needs to be made in the actual file itself, so idk.
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.
Our lint rules explicitly forbid it via Prettier (it'll collapse it again on autofix)
redirect requests to this function when the path and verb match. | ||
- `@param.body('todo', TodoSchema)` associates the OpenAPI schema for a Todo | ||
with the body of the request so that LoopBack can validate the format of an | ||
incoming request (**Note**: As of this writing, this is not yet functional). |
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.
Could you clarify that it's VALIDATION that's not functional? One could mistakenly think that the decorator itself might not be functional without context.
|
||
You can use these and other decorators to create a REST API for a full set of | ||
verbs: | ||
|
||
#### src/controllers/todo.controller.ts | ||
```ts | ||
import {post, param, get, put, patch, del} from '@loopback/openapi-v2'; |
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 line break between the constructor and @post
would be good
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.
✅✅✅
f6d67cf
to
a39b4d4
Compare
that folder, create a new file called `db.datasource.ts`. | ||
that folder, create a new file called `db.datasource.ts`. This file will create | ||
a strongly-typed export of our datasource using the `DataSourceConstructor`, | ||
which we can consume in our application via injection. | ||
|
||
#### src/datasources/db.datasource.ts | ||
|
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.
unused fs and juggler import
I noticed that some of the codes in the README are different from the actual codes (see datasources.json for example). Is this an oversight or are we conforming the real code to reflect the one in README? |
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.
In either the README.md or Step 1 (My preference is README), please add an overview of what is the tutorial application that the user is creating. It says it's a LoopBack 4 example but not what it is (a simple Todo Application) ... and this would be a good location to show how they can see the running example by using lb4 example
and then follow the tutorial for step by step instruction on re-creating the application themselves.
Can't leave a comment here but the code in Step 3 is outdated. New projects now use BootMixin
.
Overall a great PR!! This is definitely along the lines of what I have in mind for a simple and easy to follow tutorial. Lets make it simpler where we can.
and follow the on-screen prompts: | ||
``` | ||
$ lb4 | ||
``` | ||
|
||
<!-- TODO: Add screenshot of terminal here to illustrate what we mean. --> |
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.
For the purposes of this tutorial I think it's important to walk the user through all the options they should select and names they should enter to follow along with the tutorial so they don't run into issues later. This can be done in writing (copy pasting terminal output) or via a screenshot (if that's what the screenshot is intended to imply).
To generate your application using the toolkit, run the `lb4` command | ||
and follow the on-screen prompts: | ||
``` | ||
$ lb4 |
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.
Should we be using lb4 app
explicitly?
|
||
If you'd like to continue the tutorial, then move to the | ||
[Create your app scaffolding](2-scaffold-app.md) section. | ||
|
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.
Add a separator line here. For someone looking to follow this tutorial, the below adds noise imo. Change text to something like:
Continue the tutorial by visiting the next section ...
or something along those lines.
Better yet. Move the below instructions to README.md
! Keep the tutorial focused on the step by step instructions.
|
||
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 | ||
- [Node.js](https://nodejs.org/en/) at v8.x or greater | ||
|
||
Additionally, this tutorial assumes that you are comfortable with | ||
certain technologies, languages and concepts. | ||
- JavaScript (ES6) | ||
- [npm](https://www.npmjs.com/) |
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.
Don't think npm
or REST
are pre-requisites. And technically we assume basic TypeScript knowledge not Javascript (yea I know one is a superset of the other ... but syntax used in this tutorial is TS).
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.
npm
is most definitely a prerequisite, IMO
At no point have we tested our application using yarn
, and unless we do, we shouldn't tell others that it works.
I highly doubt yarn
will cause problems, but why would we tell people when we haven't tried it at least once to be sure?
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.
With respect to the REST portion being a prerequisite, I would say that this tutorial does "gloss over" some of the finer points of a REST API.
Not knowing anything about what REST is, or what HTTP verbs are and how to use them would mean that large portions of the end result of the tutorial would escape some beginners. ("What the heck is POST? Do I just type POST in front of the URL in Chrome/Firefox/Safari/etc?")
Granted, we do tell users to make use of the API Explorer, but knowing why it's not required for testing your API or interfacing with it is important, though it may seem obvious to you and I.
There's a lot of background knowledge required to understand REST so that you can get the most out of this tutorial, as well as see the value of LoopBack from an API point of view.
|
||
It also provides many of the functions and interfaces we'll require for setting | ||
up our new LoopBack application, which is why we're starting here. | ||
|
||
Jump into the directory for your new application. You'll see a folder structure | ||
similar to this: | ||
``` |
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 @shimks meant that the previous step should have a table explaining the generated app for the user.
|File/Folder|Description|
| dist/|Folder containing the compiled application code (TS -> JS)|
|README.md|Description and instructions for your Application|
I think that would be a good idea and it would eliminate the need for the file structure on this page.
operations against our database (or any other kind of datasource). | ||
|
||
We'll also inject our datasource so that this repository can connect to it when | ||
executing data operations. | ||
|
||
#### src/repositories/todo.repository.ts |
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 code formatting of this file for the imports
is different! Spaces in brackets at the start and end. Small nit-pick but lets be consistent throughout with the programming style to make it easier for users.
@@ -6,10 +14,14 @@ create two files: | |||
- `todo.repository.ts` | |||
|
|||
Our TodoRepository will contain a small base class that uses the |
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 extend
instead of will contain
.
|
||
The `@repository` decorator will retrieve and inject an instance of the | ||
`TodoRepository` whenever an inbound request is being handled. The lifecycle | ||
of your controller object is typically a per-request lifecycle, which means that |
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 would remove the word typically
. I know you cover changing the BindingScope of controllers in the NOTE below but I would propose remove the note entirely. This is a tutorial to create an app, let's keep it simple. Changing scope for controllers should be covered under the controller 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 was explaining it so that users wouldn't wonder why we're injecting the repository instead of instantiating it, which led into explaining why we usually instantiate the controller instead of injecting it. :P
[boot module](https://github.com/strongloop/loopback-next/tree/master/packages/boot) | ||
will automatically discover our controllers, repositories, datasources and | ||
other artifacts and inject them into our application for use. | ||
In the current example, we've manually loaded our repository so that it can be |
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.
repositories are also bootable now. I'm not sure of the point of mentioning mocking / stubbing for unit testing purposes 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.
At the time I wrote this, that wasn't the case!
|
||
### Navigation | ||
Next, you can use the [API Explorer](http://loopback.io/api-explorer/?url=http://localhost:3000/swagger.json) to browse your API and make requests! |
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.
Change this link to http://localhost:300/swagger-ui
so it's easier for users to understand and follow along.
2f1d52a
to
fb5872d
Compare
this.options && this.options.datasource | ||
? new DataSourceConstructor(this.options.datasource) | ||
: db; | ||
this.bind('datasource').to(datasource); |
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 we still need this binding
fb5872d
to
198d30b
Compare
16982cc
to
9a19773
Compare
@virkt25 PTAL |
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.
Make these changes and you're good
this.setupRepositories(); | ||
} | ||
|
||
setupRepositories() { |
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.
setupDatasources() {
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.
D'oh.
? new DataSourceConstructor(this.options.datasource) | ||
: db; | ||
this.bind('datasource').to(datasource); | ||
this.repository(TodoRepository); |
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.
remove this line
9a19773
to
b86c116
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.
It's looking so much better! Just a few more comments
Can't comment on the line but I think this paragraph (https://github.com/strongloop/loopback-next/pull/1056/files#diff-e2a45dfd021d8079e748e134a388dd48R35) doesn't belong in this file (they don't need to know about ping.controller
and most certainly not in the Adding Juggler section. Maybe make a note in the Controller
section stating that a default controller is provided to allow a user to test the application.
tslint.json | ||
``` | ||
|
||
| File | Purpose | |
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.
Something is causing this table to not render correctly. PTAL. Check by hitting the View
button to see the rendered markdown. Didn't spot what was causing the issue myself.
| test/ping.controller.test.ts | An example test to go with the ping controller in `src/controllers`. | | ||
|
||
<!-- | ||
kjdelisle: Why are we automatically generating MIT license files for |
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.
+1 to this. But I think this comment should be removed and a new issue should be created to capture this with a note to update this tutorial once CLI has been fixed.
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.
#1079 is now open for this problem, and I'll push the changes to remove the comment soon.
The Legacy Juggler is a "bridge" between the existing | ||
[loopback-datasource-juggler](https://github.com/strongloop/loopback-datasource-juggler) | ||
and the new LoopBack 4 architecture. It provides the capabilities required to | ||
access datasources, and perform |
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.
Include a one line of what a datasource is ... or better yet, remove that word and use something like, provides a persistence (storage) layer for your data.
other artifacts and inject them into our application for use. | ||
|
||
>**NOTE**: The boot module will only discover and inject artifacts that | ||
follow our established conventions for artifact directories. Here are some |
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 is incorrect. It follows our conventions by default but this config is exposed to the user and they can change it to their own conventions.
) { | ||
constructor(options?: ApplicationConfig) { | ||
super(options); | ||
this.projectRoot = __dirname; |
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.
New projects also list the conventions here to show users the defaults and what they can change.
|
||
Feel free to look around in the application's code to get a feel for how it | ||
works, or if you're still interested in learning how to build it step-by-step, | ||
then continue with this tutorial! | ||
|
||
### Stuck? | ||
Check out our [Gitter channel](https://gitter.im/strongloop/loopback) and ask |
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 can't leave comments for things that are not changed, but for Bugs/Feedback
section below, change the line to just read Open an issue in this repository ...
(since it's now part of the monorepo :)).
b86c116
to
a5c1795
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.
Awesome tutorial with clear steps btw!
@slnode test please |
a5c1795
to
1ff5f57
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.
Last few changes ...
|
||
Additionally, this tutorial assumes that you are comfortable with | ||
certain technologies, languages and concepts. | ||
- JavaScript (ES6) |
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.
Should this be JavaScript or TypeScript?
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.
JavaScript.
You can get away with not knowing TypeScript for this tutorial, since we give you complete working examples. Not knowing JavaScript could be a bit more problematic for consuming this tutorial.
|
||
It also provides many of the functions and interfaces we'll require for setting | ||
up our new LoopBack application, which is why we're starting here. | ||
|
||
Jump into the directory for your new application. You'll see a folder structure | ||
similar to this: | ||
``` |
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 folder structure doesn't match the one on the previous step. I think it's best to remove this entirely since the folder structure is shown in the previous step with detailed explanations! (Awesome job with that btw)
1ff5f57
to
1f82b10
Compare
on those sources of data. | ||
|
||
It also provides many of the functions and interfaces we'll require for setting | ||
up our new LoopBack application, which is why we're starting here. | ||
|
||
The application template comes with a controller, and some default wireup in | ||
`src/application.ts` that handles the basic configuration for your 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.
I thought you had removed ping.controller.ts
mention here?
e14b257
to
fca6899
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.
Awesome Tutorial! Great job 💯
fca6899
to
c6366b9
Compare
connected to #988