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 could have known earlier you were going to throw away.
Jonathan G On 10/1/19, 3:17 PM, "ocket 8888" <[email protected]> wrote: I agree, linting shouldn't be a part of package building. On Tue, Oct 1, 2019 at 12:26 PM Hoppal, Michael <[email protected]> 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/master/infrastructure/test > seems like a better place to put the linter in. > > Michael > > On 10/1/19, 12:00 PM, "Dan Kirkwood" <[email protected]> wrote: > > 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 < > [email protected]> > wrote: > > > 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" <[email protected]> > wrote: > > > > +1, I'm generally in favor of shared linters and formatters where > > possible, and that rollout path sounds good to me. > > > > - Rawlin > > > > On Mon, Sep 30, 2019 at 8:28 AM Hoppal, Michael > > <[email protected]> wrote: > > > > > > Hi, > > > > > > As we grow our Golang footprint within ATC we should consider > the > > addition of a linter for our CI. > > > > > > As with any linter it provides a lot of benefits including > enforcing > > a consistent style, early detection of potential bugs and speed up > of PR > > reviews. > > > > > > That being said I propose that we add the linter GoLangCI-Lint< > > > https://protect2.fireeye.com/url?k=bd4100fc-e1a50e37-bd412748-000babff3540-012a587242ed1320&u=https://github.com/golangci/golangci-lint> > to our CI. It wraps many > > widely used linters in the Golang opensource community with the > ability to > > turn on which ones are run. It also supports outputting results in > > checkstyle which is consumable via Jenkins for a visual report. > > > > > > To start I would recommend to stay with the default enabled > linters< > > > https://protect2.fireeye.com/url?k=9e1436b4-c2f0387f-9e141100-000babff3540-944aacd53629deee&u=https://github.com/golangci/golangci-lint#enabled-by-default-linters> > on > > the tool with the addition of Gofmt. > > > > > > The roll out path (up for discussion of course) would be: > > > > > > > > > * Makefile target within the source code to allow > developers to > > run the linting locally as they develop > > > * Inclusion of GolangCI-Lint within CI as a non-voting > component > > on every PR (as to not block development when turned on) > > > * Fixing of the current lint violations > > > * Make the linting a blocking voting component of CI > > > > > > What are peoples thoughts on the inclusion of linting in > general, > > choice of linter and the outlined rollout plan? > > > > > > Thanks, > > > > > > Michael Hoppal > > > > > > > > > > > > > > >
