-
Notifications
You must be signed in to change notification settings - Fork 37
feat: support --ignore-error #17
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
Conversation
默认还是支持吧 |
那就要做一个命令行参数出去?那也行,明早改改
发自我的 iPhone
… 在 2017年12月7日,21:01,Haoliang Gao ***@***.***> 写道:
默认还是支持吧
—
You are receiving this because you authored the thread
Reply to this email directly, view it on GitHub, or mute the thread.
|
嗯,加了这个参数不 exit 就好了 |
@popomore 改为 |
lib/cmd/start.js
Outdated
} | ||
|
||
* checkStatus({ stderr, timeout }) { | ||
* checkStatus({ stderr, timeout, 'ignore-error': ignoreError }) { |
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.
前面有个 removeAlias 干掉了它
if (stat && stat.size > 0) { | ||
const [ stdout ] = yield exec('tail -n 100 ' + stderr); | ||
this.logger.error(stdout); | ||
this.logger.error('Start failed, see %s', stderr); |
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.
这里应该不用改吧,�只是根据参数判断 isSuccess 是否返回 true
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.
这个算是风格问题吧,感觉放到外面好点吧,while 里面只是循环 stat
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.
主要这里有两个标志位,以后加逻辑比较容易错。
建议改成 ignore-stderr |
可以 |
改为 |
6843537
to
26a8058
Compare
@popomore 你发吧 |
|
// nothing | ||
} | ||
isSuccess = ignoreStdErr; | ||
this.logger.error('Start got error, see %s', stderr); |
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.
喔喔,忘记你们还继承了,下次注意
Checklist
npm test
passesAffected core subsystem(s)
Description of change