-
Notifications
You must be signed in to change notification settings - Fork 15
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
fix(gofmt): fix the issue where the gofmt stderr is not handled #171
Conversation
✅ Deploy Preview for reviewbot-x canceled.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #171 +/- ##
==========================================
- Coverage 37.61% 37.18% -0.44%
==========================================
Files 10 10
Lines 856 866 +10
==========================================
Hits 322 322
- Misses 505 515 +10
Partials 29 29 ☔ View full report in Codecov by Sentry. |
fix #169 |
if c.Stderr != nil { | ||
return nil, nil, errors.New("exec: Stderr already set") | ||
} | ||
var stdoutBuffer bytes.Buffer |
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.
要单独定义stdout/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.
1、gofmt 检查出的lint问题是存储在stdout中,
2、而像expected 'package' 错误是会在stderr中体现,
3、像找不到文件路径执行gofmt 报错则会出现在err := c.Run() 中。
若直接使用combineoutput方法则会将stdout和stderr内容同时输出到一个Buffer中,不易区分
因此这里其实是改写了combineoutput,让其分别返回stdout和stderr,分别处理
c := exec.Command(command, args...) | ||
c.Dir = dir | ||
log.Printf("final command: %v \n", c) | ||
return c.Output() | ||
if c.Stdout != nil { |
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.
这里应该没必要做这种检查
No description provided.