Re: [EXTERNAL] Re: Inclusion of Golang Linter in CI

2019-10-02 Thread Rawlin Peters
> but we shouldn't forget the java code when we're talking about linting. I've used pmd on java in the distant past, to reasonable effect, but I don't have a super-strong opinion on the matter. FWIW we do use PMD for Traffic Router, but I don't know off the top of my head if it's checked on every

Re: [EXTERNAL] Re: Inclusion of Golang Linter in CI

2019-10-02 Thread Dan Kirkwood
+1 On Tue, Oct 1, 2019 at 4:32 PM ocket wrote: > I don't think we should get bogged down with the configuration right now, > that should appear in a PR at some point which will be much easier to use > as a platform for the debate. > > I don't mean to suggest that `pkg` wouldn't run the lint

Re: [EXTERNAL] Re: Inclusion of Golang Linter in CI

2019-10-01 Thread Chris Lemmons
And to be sure, Michael's roll out timeline is exactly right. Add a container that runs all the applicable linters, outputs the test results, and then exits 0, regardless of the result. That makes the build succeed, but produces the error list so we can work on it. Once we get it all cleared out, w

Re: [EXTERNAL] Re: Inclusion of Golang Linter in CI

2019-10-01 Thread ocket 8888
I don't think we should get bogged down with the configuration right now, that should appear in a PR at some point which will be much easier to use as a platform for the debate. I don't mean to suggest that `pkg` wouldn't run the linter, just that its doing so would be as a consequence of building

Re: [EXTERNAL] Re: Inclusion of Golang Linter in CI

2019-10-01 Thread Robert Butts
I think `pkg` is the right place to put this. `pkg` isn't just for building. I think it has that name because it's short, not because it's limited to "packaging." It also does things like the license checking via Weasel. It's more like `make`, which also isn't limited to building. It's really for

Re: [EXTERNAL] Re: Inclusion of Golang Linter in CI

2019-10-01 Thread Chris Lemmons
Yeah, that's the name, but all it does is start the jobs in infrastructure/docker/build/docker-compose.yml . Adding it to the build infrastructure is the right way to make it easy for everyone to run it in the exact same way, including the CI. The pkg script is just a helper that reduces the requi

Re: [EXTERNAL] Re: Inclusion of Golang Linter in CI

2019-10-01 Thread Gray, Jonathan
I would lean towards running any kind of linter before or as part of compilation because the answer doesn't depend on the output of code. It's not a huge difference when it's not enforced, but if it someday becomes a mandatory criterion it's better not to burn the compute time on something you

Re: [EXTERNAL] Re: Inclusion of Golang Linter in CI

2019-10-01 Thread ocket 8888
I agree, linting shouldn't be a part of package building. On Tue, Oct 1, 2019 at 12:26 PM Hoppal, Michael wrote: > `pkg` seems like a weird location for a linter to me. It doesn’t have > anything to do with packaging/building of the services. > > https://github.com/apache/trafficcontrol/tree/mas

Re: [EXTERNAL] Re: Inclusion of Golang Linter in CI

2019-10-01 Thread Hoppal, Michael
`pkg` seems like a weird location for a linter to me. It doesn’t have anything to do with packaging/building of the services. https://github.com/apache/trafficcontrol/tree/master/infrastructure/test seems like a better place to put the linter in. Michael On 10/1/19, 12:00 PM, "Dan Kirkwood"

Re: [EXTERNAL] Re: Inclusion of Golang Linter in CI

2019-10-01 Thread Dan Kirkwood
It really should only be an addition to `infrastructure/docker/build/docker-compose.yml` as `pkg` just passes its arguments to `docker-compose`. -dan On Tue, Oct 1, 2019 at 10:33 AM Gray, Jonathan wrote: > How do you think the linter process would integrate with our existing > ./pkg wrapper if

Re: [EXTERNAL] Re: Inclusion of Golang Linter in CI

2019-10-01 Thread Gray, Jonathan
How do you think the linter process would integrate with our existing ./pkg wrapper if at all? Jonathan G On 10/1/19, 10:24 AM, "Rawlin Peters" wrote: +1, I'm generally in favor of shared linters and formatters where possible, and that rollout path sounds good to me. - Rawlin

Re: [EXTERNAL] Re: Inclusion of Golang Linter in CI

2019-10-01 Thread ocket 8888
Okay, the bit about consistent output formatting is a good point. I suppose any of my concerns are best addressed in the configuration, really. +1 On Tue, Oct 1, 2019, 08:53 Jeremy Mitchell wrote: > As a Go noob, I defer to the experts... > > On Mon, Sep 30, 2019 at 9:05 PM Dan Kirkwood wrote:

Re: [EXTERNAL] Re: Inclusion of Golang Linter in CI

2019-10-01 Thread Jeremy Mitchell
As a Go noob, I defer to the experts... On Mon, Sep 30, 2019 at 9:05 PM Dan Kirkwood wrote: > Having used golangci-lint a bit, I can tell you that it's configurable to > disable individual checkers via a config file and is quite fast. I'm of > the opinion that having the redundancy might be a

Re: [EXTERNAL] Re: Inclusion of Golang Linter in CI

2019-09-30 Thread Dan Kirkwood
Having used golangci-lint a bit, I can tell you that it's configurable to disable individual checkers via a config file and is quite fast. I'm of the opinion that having the redundancy might be an issue if it were inefficient, but it's not and these linters are likely to evolve over a short time

Re: [EXTERNAL] Re: Inclusion of Golang Linter in CI

2019-09-30 Thread Hoppal, Michael
Great input! I think the beauty of golangci-lint is we can organically iterate overtime and easily define the right set of linters that we want to use all while keeping consistent output formatting that is easily consumable by Jenkins. If we think there is too much overlap in the defaults we cou