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: support some new features #5
Conversation
@gxcsoccer, thanks for your PR! By analyzing the history of the files in this pull request, we identified @fengmk2 and @dead-horse to be potential reviewers. |
Current coverage is 100% (diff: 100%)
|
3df0d3d
to
f8e5c9b
Compare
.then(() => this.ready(true)) | ||
.catch(err => this.ready(err)); | ||
} | ||
this._ready = false; |
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.
ready 放到 get-ready 实现吧?
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.ready(err)
会同时 emit error,这就要求 client 是一个 EventEmitter,所以放 get-ready 反而有点不合适了
@@ -0,0 +1,21 @@ | |||
MIT License | |||
|
|||
Copyright (c) dead_horse and other contributors |
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.
Copyright (c) node-modules and other contributors
return; | ||
} | ||
console.error('\n[%s][pid: %s][%s][%s] %s: %s \nError Stack:\n %s', | ||
Date(), process.pid, this.constructor.name, __filename, err.name, |
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.
__filename 其实不准确的。。。
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.
__filename 可以不打印
|
||
var ready = require('get-ready'); |
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.
为啥不在 get-ready 实现 ready 的逻辑?
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.
以后就都 extends SDKBase 吧,我看了下 get-ready 其实也就是我们的几个模块在用
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.
ok
f8e5c9b
to
cae054f
Compare
cae054f
to
680ed64
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.
+1
3.0.0 |
close eggjs/egg#175