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(middleware.md): translated & proofread 'middleware.md' #784

Merged
merged 8 commits into from
May 10, 2017

Conversation

lslxdx
Copy link
Contributor

@lslxdx lslxdx commented Apr 20, 2017

'middleware.md' is translated by @lslxdx and proofread by @闷油瓶小张

related issue: #783

Checklist
  • documentation is changed or added
Affected core subsystem(s)
Description of change

close #783

'middleware.md' is translated by @lslxdx and proofread by @闷油瓶小张

related issue: eggjs#783
@codecov
Copy link

codecov bot commented Apr 20, 2017

Codecov Report

Merging #784 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #784   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          28     28           
  Lines         667    667           
=====================================
  Hits          667    667

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e55a134...330d0cb. Read the comment docs.


## Writing Middleware

### how to write
Copy link
Member

Choose a reason for hiding this comment

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

capitalize how

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. commit: fefcbdb


### how to write

We begin by writing a simple gzip middleware, to see how to write middleware
Copy link
Member

Choose a reason for hiding this comment

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

We begin to see how to write a middleware from a simple gzip example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. commit: fefcbdb


### Configuration

Usually the middleware has its own configuration. In the framework, a complete middleware is including the configuration process. We agree that a middleware is a separate file placed in `app/middleware` directory, which needs an exports function that take two parameters:
Copy link
Member

@popomore popomore Apr 20, 2017

Choose a reason for hiding this comment

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

a middleware is a file in app/middleware directory by convention

convention from https://en.wikipedia.org/wiki/Convention_over_configuration

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. commit: fefcbdb

## Importing Middleware in the Application

We can import customized middleware completely by configuration in the application, and decide their order.
If we need to import the gzip Middleware in the above,
Copy link
Member

Choose a reason for hiding this comment

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

Middleware > middleware

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. commit: fefcbdb

},
};
```
** Note: middleware imported by the framework and plugins are loaded earlier than those imported by the application layer, and the application layer cannot overwrite the default framework middleware. If the application layer imports customized middleware that has the same name with default framework middleware, an error will be raised on starting up. **
Copy link
Member

@popomore popomore Apr 20, 2017

Choose a reason for hiding this comment

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

better to change layer to middleware

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change 'application layer' to 'application middleware'?

How about changing 'application layer' to 'application'? Does it make sense?

Copy link
Member

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good!
Anyway, it is not finalized and can be changed at any time though.

Does anyone have other suggestions?

```
** Note: middleware imported by the framework and plugins are loaded earlier than those imported by the application layer, and the application layer cannot overwrite the default framework middleware. If the application layer imports customized middleware that has the same name with default framework middleware, an error will be raised on starting up. **

## The Use of Middleware in Router
Copy link
Member

Choose a reason for hiding this comment

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

- The Use of Middleware in Router
+ Middleware in Router

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. commit: fefcbdb

@popomore
Copy link
Member

感觉中文文档可以把引入改为加载,翻译成 import 有点难懂,还是叫 load 比较好。

@lslxdx
Copy link
Contributor Author

lslxdx commented Apr 21, 2017

@popomore

感觉中文文档可以把引入改为加载,翻译成 import 有点难懂,还是叫 load 比较好。

需要在英文文档中修改过来吗?还是说先改中文文档,再改英文文档?

@popomore
Copy link
Member

需要在英文文档中修改过来吗?还是说先改中文文档,再改英文文档?

中英文一起改吧,看看其他人的意见 @dead-horse @atian25

@atian25
Copy link
Member

atian25 commented Apr 21, 2017 via email

@lslxdx
Copy link
Contributor Author

lslxdx commented Apr 21, 2017

@popomore @atian25

可以做3件事情:

  1. 创建一个单独的issue, 比如中文文档应该把引入改为加载,因为翻译成 import 有点难懂,还是叫 load 比较好。
  2. 在这个doc的翻译版上改成"load"
  3. 在这个doc的中文版上改成"加载"

可行吗?哪几件(些)事情可行的?

@popomore
Copy link
Member

@lslxdx 在这篇文章里面把中英文都改了好了,其他文章遇到翻译不通顺的地方可以选择性修改。

@lslxdx
Copy link
Contributor Author

lslxdx commented Apr 21, 2017

@popomore 好的,晚上改

lslxdx pushed a commit to lslxdx/egg that referenced this pull request Apr 21, 2017
将这篇文档中的'引入'更改为'加载', 这样可以更容易理解

issue: eggjs#784
Change 'import' to 'load' for a better understanding.

issue: eggjs#784
lslxdx added 2 commits April 24, 2017 10:08
Change "Loading Middleware in the Application" to "Using Middleware in the Application"
@lslxdx
Copy link
Contributor Author

lslxdx commented Apr 24, 2017

@popomore IIRC, we discussed the "application layer/middleware" issue days ago(I can't find the discussion URL, sigh), luckily I find a similar expression here just now, for example:

You can load application-level and router-level middleware with an optional mount path.

and

Application-level middleware

So could we use "application-level" as an agreement?

@popomore
Copy link
Member

When we talk about middleware, we have two steps

  1. First middelwares will be loaded from application, plugin and framework, it can be called application-level middleware. build-in middleware also meam framework-level middleware.
  2. Then I use the middleware which have been loaded, it can be used automatically by configuration whitch is config.appMiddleware and config.coreMiddleware, it also can be used in router manually. That's what express does.

So I agree using xx-level in document of load middleware, but don't use it in use middleware.

@lslxdx
Copy link
Contributor Author

lslxdx commented Apr 26, 2017

So I agree using xx-level in document of load middleware, but don't use it in use middleware.

Yes, that makes sense and I agree too!

@atian25
Copy link
Member

atian25 commented May 3, 2017

Is there anything still block this pr, guys?

@popomore
Copy link
Member

@lslxdx can you complete this PR first?

@lslxdx
Copy link
Contributor Author

lslxdx commented May 10, 2017

@popomore I'm stuck on this thread and could you please help me out?

All I can see is:

Review required

At least one approved review is required by reviewers with write access. Learn more.

All checks have passed

5 successful checks

Merging is blocked

Merging can be performed automatically with one approved review.

What should I do next?

@popomore
Copy link
Member

So I agree using xx-level in document of load middleware, but don't use it in use middleware.

Yes, that makes sense and I agree too!

I think you will update something, maybe I was wrong?

@lslxdx
Copy link
Contributor Author

lslxdx commented May 10, 2017

Aha! Got it and I am on it!

Use 'application-level' instead of 'application layer' in some cases.
@lslxdx
Copy link
Contributor Author

lslxdx commented May 10, 2017

@popomore I make some changes just now and please have a look.

Copy link
Member

@popomore popomore left a comment

Choose a reason for hiding this comment

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

LGTM

@popomore popomore merged commit 22c9cd9 into eggjs:master May 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants