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

fix(gofmt): fix the issue where the gofmt stderr is not handled #171

Merged
merged 1 commit into from
Jun 17, 2024

Conversation

wwcchh0123
Copy link
Contributor

No description provided.

Copy link

netlify bot commented Jun 14, 2024

Deploy Preview for reviewbot-x canceled.

Name Link
🔨 Latest commit 7a89a79
🔍 Latest deploy log https://app.netlify.com/sites/reviewbot-x/deploys/666c23a00031170008a515c4

Copy link

codecov bot commented Jun 14, 2024

Codecov Report

Attention: Patch coverage is 0% with 16 lines in your changes missing coverage. Please review.

Project coverage is 37.18%. Comparing base (6e2f76e) to head (7a89a79).

Files Patch % Lines
internal/linters/go/gofmt/gofmt.go 0.00% 16 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

@wwcchh0123
Copy link
Contributor Author

fix #169

if c.Stderr != nil {
return nil, nil, errors.New("exec: Stderr already set")
}
var stdoutBuffer bytes.Buffer
Copy link
Contributor

Choose a reason for hiding this comment

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

要单独定义stdout/stderr 的原因是?

Copy link
Contributor Author

@wwcchh0123 wwcchh0123 Jun 17, 2024

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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

这里应该没必要做这种检查

@CarlJi CarlJi merged commit 0bc3810 into qiniu:master Jun 17, 2024
7 of 9 checks passed
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.

2 participants