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 requirements for
building so that you don't have to keep docker-compose on hand,
handles the return code of the processes, and organizes the log output
into something you can read.
golangci-lint is the gold standard metalinter for the community. In
general, if it's complaining about something, it's probably doing so
with good reason. It is the blessed replacement for gometalinter,
which was the previous gold standard. I'm +1 on golangci-lint.
We will want to have additional discussions about which specific
linters we turn on or off. Some of our existing frequent practices are
at odds with a few of the linters and we'll want to decide whether our
practices or the linter's suggestions are better. (Shadowed variables,
I'm looking at you. :) )
I like having it output the errors as junit-xml for the CI, because
then we can list that as a unit test file and it will make a great
interface for seeing what is failing and when. ("Unit test" is a bit
of a misnomer here, but all the concepts map just fine. Call it a
"static unit test" or something.)
Add it as an item in the build/docker-compose and people can run it
manually or just as part of a build. We don't really want to encourage
skipping the static analysis when doing a full build, anyway. (It is,
of course, still possible, the same way you can skip the weasel if you
really wanted.)
We've also got java and perl. Perl is on its way out, so I don't know
that adding a linter and fixing issues there is a great use of energy,
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.
On Tue, Oct 1, 2019 at 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
> > > >
> > > >
> > >
> > >
> > >
> >
> >
> >
