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

feat: remove 4.x support & fix test #3

Merged
merged 1 commit into from
Feb 21, 2017
Merged

feat: remove 4.x support & fix test #3

merged 1 commit into from
Feb 21, 2017

Conversation

atian25
Copy link
Member

@atian25 atian25 commented Feb 20, 2017

Checklist
  • npm test passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
Description of change

@mention-bot
Copy link

@atian25, thanks for your PR! By analyzing the history of the files in this pull request, we identified @huacnlee to be a potential reviewer.

@codecov-io
Copy link

codecov-io commented Feb 20, 2017

Codecov Report

Merging #3 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master     #3   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           4      4           
  Lines          10      6    -4     
=====================================
- Hits           10      6    -4
Impacted Files Coverage Δ
app/middleware/conditional.js 100% <100%> (ø)
app/middleware/etag.js 100% <100%> (ø)

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 1dc4ba8...f9fea01. Read the comment docs.

.autod.conf.js Outdated
@@ -4,8 +4,8 @@ module.exports = {
write: true,
prefix: '^',
test: [
'test',
'benchmark',
'test',
Copy link
Member

Choose a reason for hiding this comment

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

缩进少了一个空格

README.md Outdated
@@ -22,6 +22,8 @@

Wrap [koa-etag](https://github.com/koajs/etag) and [koa-conditional-get](https://github.com/koajs/conditional-get) for egg

**We recommend to use nginx to do this for better performance rather than use this plugin.**
Copy link
Member

Choose a reason for hiding this comment

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

[nginx etag](http://nginx.org/en/docs/http/ngx_http_core_module.html#etag)

@@ -7,22 +7,21 @@
},
"keywords": [
"egg",
"eggPlugin",
"egg-plugin"
Copy link
Member

Choose a reason for hiding this comment

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

为啥删除这些关键词?不是方便 npm 搜索吗?

Copy link
Member

@fengmk2 fengmk2 left a comment

Choose a reason for hiding this comment

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

其他没问题了

README.zh_CN.md Outdated
@@ -26,6 +26,8 @@

此功能在一些内容变化不多的场景,能有效避免网络下载。

**通常来说,基于性能方面的考虑,在生产环境我们会使用 nginx 替代该插件来计算 etag 。**
Copy link
Collaborator

Choose a reason for hiding this comment

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

nginx -> Nginx
etag -> ETag

@atian25 atian25 force-pushed the remove-4 branch 2 times, most recently from f54f559 to 4c8f779 Compare February 21, 2017 02:12
@atian25
Copy link
Member Author

atian25 commented Feb 21, 2017

done, review again

@atian25
Copy link
Member Author

atian25 commented Feb 21, 2017

ping @huacnlee, need more +1

README.md Outdated
@@ -22,6 +22,8 @@

Wrap [koa-etag](https://github.com/koajs/etag) and [koa-conditional-get](https://github.com/koajs/conditional-get) for egg

**For better performance, we recommend to use [Nginx Etag](http://nginx.org/en/docs/http/ngx_http_core_module.html#etag) rather than use this plugin.**
Copy link
Collaborator

Choose a reason for hiding this comment

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

Etag -> ETag

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@huacnlee huacnlee merged commit 278e0d2 into master Feb 21, 2017
@huacnlee huacnlee deleted the remove-4 branch February 21, 2017 05:47
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