-
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
feat: copy linter binary to container #325
Conversation
✅ Deploy Preview for reviewbot-x canceled.
|
4cb2f5d
to
06ceb89
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #325 +/- ##
==========================================
- Coverage 37.24% 37.20% -0.04%
==========================================
Files 27 27
Lines 1995 2013 +18
==========================================
+ Hits 743 749 +6
- Misses 1157 1165 +8
- Partials 95 99 +4 ☔ View full report in Codecov by Sentry. |
config/config.go
Outdated
@@ -38,13 +38,23 @@ type GlobalConfig struct { | |||
JavaStyleCheckRuleConfig string `json:"javastylecheckruleConfig,omitempty"` | |||
} | |||
|
|||
type LogStorageConfig struct { |
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.
没用到的先删掉
config/config.go
Outdated
CustomRemoteConfigs map[string]any `json:"customRemoteConfigs"` | ||
} | ||
|
||
type GithubConfig struct{} |
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.
同上
config/config.go
Outdated
type GithubConfig struct{} | ||
|
||
type DockerAsRunner struct { | ||
Image string `json:"image,omitempty"` |
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.
DockerAsRunner 配置若存在,Image这里是不是应该必须要有?
internal/runner/docker.go
Outdated
log.Infof("container created: %v", resp.ID) | ||
|
||
if cfg.DockerAsRunner.CopyLinterFromOrigin { | ||
cmd := exec.Command("which", lintername) |
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.
internal/runner/docker.go
Outdated
linterOrignPath = string(cmdOutput[:len(cmdOutput)-1]) | ||
} | ||
if err != nil { | ||
log.Errorf("failed to find %s :%v", lintername, err) |
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.
error 为何都不return呢?
internal/runner/runner.go
Outdated
@@ -46,7 +46,7 @@ func (l *LocalRunner) Prepare(ctx context.Context, cfg *config.Linter) error { | |||
return nil | |||
} | |||
|
|||
func (l *LocalRunner) Run(ctx context.Context, cfg *config.Linter) (io.ReadCloser, error) { | |||
func (l *LocalRunner) Run(ctx context.Context, cfg *config.Linter, lintername string) (io.ReadCloser, error) { |
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.
config.Linter 还是增加个属性Name,来做这个事情
Agent.LinterName 可以相应的删掉
334a819
to
47997eb
Compare
47997eb
to
e960ca0
Compare
No description provided.