goheader: fix relative template path #3748
Conversation
|
Hey, thank you for opening your first Pull Request ! |
|
|
thejan2009
force-pushed
the
master
branch
2 times, most recently
from
2200503
to
bf97e8d
Compare
ldez
added
linter: update
|
Hi, is there anything I can help with to unblock this PR? I saw #3682 related to denis-tingaikin/go-header#22. Should that issue be fixed first? |
|
@ldez bump |
test/testdata/configs/go-header.yml
Outdated
| @@ -1,6 +1,6 @@ | |||
| linters-settings: | |||
| goheader: | |||
| template: MY {{title}} | |||
| template-path: go-header-template | |||
There was a problem hiding this comment.
let's keep both tests
and template and template-path
(with better naming for config and test files)
Antonboom
added
feedback required
go-header template-path setting is usually a relative filepath. As a result, the linter only worked when golangci-lint was invoked from that folder. We detect if the template path is relative and prefix it with config path. go-header_bad.go test case was modified because old files are not checked as seen here https://github.com/denis-tingaikin/go-header/blob/v0.4.3/analyzer.go#L59-L63
| @@ -1,4 +1,4 @@ | |||
| /*MY TITLE!*/ // want `Expected:TITLE\., Actual: TITLE!` | |||
| /*MY TITLE?*/ // want `Expected:TITLE\., Actual: TITLE?` | |||
| @@ -0,0 +1,6 @@ | |||
| /*MY TITLE.*/ | |||
There was a problem hiding this comment.
where is go-header-template_bad?
| /*MY TITLE.*/ | ||
|
|
||
| //golangcitest:args -Egoheader | ||
| //golangcitest:config_path testdata/configs/go-header-template.yml |
There was a problem hiding this comment.
Just a question – how it works without test/ prefix?
Looks like your filepath.Join is not covered 🤔
| @@ -247,6 +247,10 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { | |||
| if stylecheckCfg != nil && stylecheckCfg.GoVersion != "" { | |||
| stylecheckCfg.GoVersion = trimGoVersion(m.cfg.Run.Go) | |||
| } | |||
|
|
|||
| if goheaderCfg != nil { | |||
| goheaderCfg.LintConfigDir = m.cfg.GetConfigDir() | |||
There was a problem hiding this comment.
I meant you can override goheaderCfg.TemplatePath at this level.
But I cannot decide which option is better
| @@ -247,6 +247,10 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { | |||
| if stylecheckCfg != nil && stylecheckCfg.GoVersion != "" { | |||
| stylecheckCfg.GoVersion = trimGoVersion(m.cfg.Run.Go) | |||
| } | |||
|
|
|||
| if goheaderCfg != nil { | |||
| goheaderCfg.LintConfigDir = m.cfg.GetConfigDir() | |||
There was a problem hiding this comment.
Did you try to run golangci-lint without config, it works as expected?
ldez
added
blocked
go-header template-path setting is usually a relative filepath. As a result, the linter only worked when golangci-lint was invoked from that folder.
We detect if the template path is relative and prefix it with config path.
go-header_bad.go test case was modified because old files are not checked https://github.com/denis-tingaikin/go-header/blob/v0.4.3/analyzer.go#L59-L63