Skip to content
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(boot): Docs for @loopback/boot #631

Merged
merged 1 commit into from
Feb 28, 2018
Merged

docs(boot): Docs for @loopback/boot #631

merged 1 commit into from
Feb 28, 2018

Conversation

virkt25
Copy link
Contributor

@virkt25 virkt25 commented Feb 21, 2018

Documentation explaining the @loopback/boot extension and the various aspects that make it up.

Need a review and suggestions on the following:
-- Should individual Booters be documented here? (Or link to docs in README for the package?)
-- Layout?
-- Grammar?

Copy link
Member

@dhmlau dhmlau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this doc might have good information on what boot is and how it works, I might miss the points (as a user):

  • why do i need to care about how boot if artifacts are automatically loaded for me when using CLI
  • in what case i need to know more about booters

* Process those artifacts automatically (this usually means binding them to the Application)

`@loopback/boot` provides a Bootstrapper that uses Booters to automatically
discover and bind artifacts, all packaged in an easy to use Mixin.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"easy to use" => "easy-to-use"?

### What is an artifact?

An artifact is any LoopBack construct usually defined in code as a Class. The word
artifact describes Controllers, Repositories, Models, etc.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion to reword the 2nd sentence:
LoopBack constructs include Controllers, Repositories.....

### app.booters()

A convenience method manually bind `Booters`. You can pass any number of `Booter`
classes to this method and they will all get bound to the Application using the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get bound => get bounded

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is be bound better?


## BootComponent

This component is added to a Application by `BootMixin` if used. The Component:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"a Application" => "an Application"?
"The Component" => "This Component"

## Bootstrapper

A Class that acts as the "manager" for Booters. The Boostrapper is designed to be
bound to an Application as a `SINGLETON`. The Bootstrapper class provides a `boot()`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bound -> bounded

method. This method is responsible for getting all bound `Booters` and running
their `phases`. A `phase` is a method on a `Booter` class.

Each call of the `boot()` method creates a new `Context` that sets the `app` context
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion:
"Each call of the boot() method" => "Each boot() method call"

their `phases`. A `phase` is a method on a `Booter` class.

Each call of the `boot()` method creates a new `Context` that sets the `app` context
as it's parent. This is done so each `Context` for `boot` gets a new instance of
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's parent => its parent


## Usage: @loopback/cli

New projects generated using `@loopback/cli` or `lb4` are automatically enabled
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it's a good idea to let the readers know up front that for general use, artifacts are automatically loaded for you and you don't need to know too much details.

Copy link
Contributor

@shimks shimks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice docs. It's missing a sidebar entry and there are some minor nitpicks, but once that's been taken care of, LGTM.

#### Using the BootMixin

`Booter` and `Binding` types must be imported alongside `BootMixin` to allow TypeScript
to infer types and avoid errors. \_If using `tslint` with the `no-unused-variable` rule,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\_

servers and bindings. The Application class extends [Context](Context.html), and
provides the controls for starting and stopping itself and its associated
servers.
* [**Booting an Application**](Booting-an-Application.html): A convention based
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the section on boot should be somewhere at the bottom if the order matters. Most users of LoopBack won't really have to know about the fine details of booters other than that they do context bindings for you. I think it'd be more appropriate below Context here.


* Discover artifacts automatically based on a convention (a specific folder
containing files with a given suffix)
* Process those artifacts automatically (this usually means binding them to the Application)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use fit in the concept of context in here somewhere, just to make sure that users understand that boot takes care of automatic context bindings.

@virkt25
Copy link
Contributor Author

virkt25 commented Feb 23, 2018

@dhmlau Good points! Thoughts on splitting this doc into 2 different pages?

  • One for "users" of the Application (all they need to know is CLI projects are enables, how to add to manual application and the config options avaiable to them)
  • Another for Extension Developers who would have more of an interest in understanding how @loopback/boot works and how they can leverage this knowledge to write their own Booter -- realistically I think an extension developer just needs to know how to write a Booter which doesn't require details about the inner workings

Does anyone need to know how Boot works under the hood?

@dhmlau
Copy link
Member

dhmlau commented Feb 23, 2018

+1 on splitting into 2 pages or at least 2 sections.
I wonder if the split is MVP. But the other argument is we might as well get it all fixed at one time, rather than going back again when we have time.

Copy link

@kjdelisle kjdelisle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent overall, just has some minor spelling, grammar and phrasing problems to address and then it can land.

Since this is a convention based Bootstrapper, it is important to set a `projectRoot`,
as all other artifact paths will be resolved relative to this path.

_Tip_: `application.ts` will likely be at the root of your project, so it's path can be

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's -> its

Components to set the property `booters` as an Array of `Booters`. They will be bound
to the Application and called by the `Bootstrapper`.

Since this is a convention based Bootstrapper, it is important to set a `projectRoot`,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

convention-based

### app.boot()

A convenience method to retrieve the `Bootstrapper` instance bound to the
Application and calls it's `boot` function. This should be called before an

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's -> its


### app.booters()

A convenience method manually bind `Booters`. You can pass any number of `Booter`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A convenience method to manually bind...

### app.booters()

A convenience method manually bind `Booters`. You can pass any number of `Booter`
classes to this method and they will all get bounded to the Application using the

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

..and they will all be bound...


## Bootstrapper

A Class that acts as the "manager" for Booters. The Boostrapper is designed to be

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Boostrapper -> Bootstrapper

:P

## Bootstrapper

A Class that acts as the "manager" for Booters. The Boostrapper is designed to be
bounded to an Application as a `SINGLETON`. The Bootstrapper class provides a `boot()`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bounded -> bound

`booters` but the same context can be passed into `boot` so selective `phases` can be
run in different calls of `boot`.

The boostrapper can be configured to run only certain booters or phases of booters

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

boostrapper -> bootstrapper

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also rephrase it a bit: ...can be configured to run specific booters, or boot phases...

by passing in `BootExecOptions`. **This is experimental and subject to change. Hence,
this functionality is not exposed when calling `boot()` via `BootMixin`**.

To use `BootExecOptions` you must directly call `bootstrapper.boot()` instead of `app.boot()`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a comma after BootExecOptions.

"To use BootExecOptions, you must..."
(I can't use the backticks in the example, messes up the formatting.)

* [**Server**](Server.html): Represents implementation for inbound transports and/or protocols such as REST over http, gRPC over http2, and graphQL over https. It typically listens for requests on a specific port, handle them, and return appropriate responses.
* [**Context**](Context.html): An abstraction of all state and dependencies in your application, that LoopBack uses to “manage” everything. It's a global registry for everything in your app (configurations, state, dependencies, classes, and so on).
* [**Dependency Injection**](Dependency-injection.html): Technique that separates the construction of dependencies of a class or function from its behavior, to keep the code loosely coupled.
* [**Booting an Application**](Booting-an-Application.html): A convention based

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

convention-based

@dhmlau
Copy link
Member

dhmlau commented Feb 23, 2018

+1 on splitting into 2 pages or at least 2 sections.
I wonder if the split is MVP. But the other argument is we might as well get it all fixed at one time, rather than going back again when we have time.

Please feel free to create separate issue to track that. I don't think it should be a blocker to land this PR.


#### configure

Used to configure the `Booter` with it's default options.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's -> its

### Example

```ts
class MyApp extends BootMixin(Application) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we show the imports here?

Copy link
Contributor

@shimks shimks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some tables are broken, but once that's been fixed, :shipit:


| Options | Type | Default | Description |
| ------------ | ------------------- | -------------------- | ------------------------------------------------------------------------------------------------------------- |
| `dirs` | `string | string[]` | `['controllers']` | Paths relative to projectRoot to look in for Controller artifacts |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The table is broken here


| Options | Type | Default | Description |
| ------------ | ------------------- | -------------------- | ------------------------------------------------------------------------------------------------------------- |
| `dirs` | `string | string[]` | `['repositories']` | Paths relative to projectRoot to look in for Repository artifacts |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Broken here as well

Copy link
Contributor

@b-admike b-admike left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for Kyu's comments, LGTM otherwise

@virkt25
Copy link
Contributor Author

virkt25 commented Feb 28, 2018

Good catch @shimks. Used | in the table without escaping it. Weird that the backticks don't escape it automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants