You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficcontrol.apache.org by "Hoppal, Michael" <Mi...@comcast.com> on 2019/09/30 21:10:23 UTC
Re: [EXTERNAL] Re: Inclusion of Golang Linter in CI
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 could pair it down to something more like:
* govet
* errcheck
* ineffassign
* gofmt
Thanks,
Michael
On 9/30/19, 2:29 PM, "ocket 8888" <oc...@gmail.com> wrote:
Let me start by saying I'm all for a linter for the Go codebase(s).
This collection of linters seems to have a lot of redundancy, and be built
with using their specific CI platform in mind. In just the default linters
it lists, it has four or five different linters to check for unused code,
something that is performed already by the compiler (type checking), and
both "govet" and something that describes itself as "govet on steroids". So
I wonder if maybe we want to pick something smaller and less redundant.
On Mon, Sep 30, 2019 at 8:28 AM Hoppal, Michael <Mi...@comcast.com>
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://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://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
>
>
>
Re: [EXTERNAL] Re: Inclusion of Golang Linter in CI
Posted by ocket 8888 <oc...@gmail.com>.
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 <mi...@gmail.com> wrote:
> As a Go noob, I defer to the experts...
>
> On Mon, Sep 30, 2019 at 9:05 PM Dan Kirkwood <da...@gmail.com> 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 an issue if it were
> > inefficient, but it's not and these linters are likely to evolve over a
> > short time..
> >
> > I'm definitely +1 on at least exploring this tool for ATC.
> >
> > -dan
> >
> > On Mon, Sep 30, 2019 at 3:10 PM Hoppal, Michael <
> > Michael_Hoppal@comcast.com>
> > wrote:
> >
> > > 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 could pair it
> > > down to something more like:
> > >
> > > * govet
> > > * errcheck
> > > * ineffassign
> > > * gofmt
> > >
> > > Thanks,
> > >
> > > Michael
> > >
> > > On 9/30/19, 2:29 PM, "ocket 8888" <oc...@gmail.com> wrote:
> > >
> > > Let me start by saying I'm all for a linter for the Go codebase(s).
> > >
> > > This collection of linters seems to have a lot of redundancy, and
> be
> > > built
> > > with using their specific CI platform in mind. In just the default
> > > linters
> > > it lists, it has four or five different linters to check for unused
> > > code,
> > > something that is performed already by the compiler (type
> checking),
> > > and
> > > both "govet" and something that describes itself as "govet on
> > > steroids". So
> > > I wonder if maybe we want to pick something smaller and less
> > redundant.
> > >
> > > On Mon, Sep 30, 2019 at 8:28 AM Hoppal, Michael <
> > > Michael_Hoppal@comcast.com>
> > > 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://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://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
> > > >
> > > >
> > > >
> > >
> > >
> > >
> >
>
Re: [EXTERNAL] Re: Inclusion of Golang Linter in CI
Posted by Jeremy Mitchell <mi...@gmail.com>.
As a Go noob, I defer to the experts...
On Mon, Sep 30, 2019 at 9:05 PM Dan Kirkwood <da...@gmail.com> 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 an issue if it were
> inefficient, but it's not and these linters are likely to evolve over a
> short time..
>
> I'm definitely +1 on at least exploring this tool for ATC.
>
> -dan
>
> On Mon, Sep 30, 2019 at 3:10 PM Hoppal, Michael <
> Michael_Hoppal@comcast.com>
> wrote:
>
> > 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 could pair it
> > down to something more like:
> >
> > * govet
> > * errcheck
> > * ineffassign
> > * gofmt
> >
> > Thanks,
> >
> > Michael
> >
> > On 9/30/19, 2:29 PM, "ocket 8888" <oc...@gmail.com> wrote:
> >
> > Let me start by saying I'm all for a linter for the Go codebase(s).
> >
> > This collection of linters seems to have a lot of redundancy, and be
> > built
> > with using their specific CI platform in mind. In just the default
> > linters
> > it lists, it has four or five different linters to check for unused
> > code,
> > something that is performed already by the compiler (type checking),
> > and
> > both "govet" and something that describes itself as "govet on
> > steroids". So
> > I wonder if maybe we want to pick something smaller and less
> redundant.
> >
> > On Mon, Sep 30, 2019 at 8:28 AM Hoppal, Michael <
> > Michael_Hoppal@comcast.com>
> > 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://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://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
> > >
> > >
> > >
> >
> >
> >
>
Re: [EXTERNAL] Re: Inclusion of Golang Linter in CI
Posted by Dan Kirkwood <da...@gmail.com>.
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..
I'm definitely +1 on at least exploring this tool for ATC.
-dan
On Mon, Sep 30, 2019 at 3:10 PM Hoppal, Michael <Mi...@comcast.com>
wrote:
> 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 could pair it
> down to something more like:
>
> * govet
> * errcheck
> * ineffassign
> * gofmt
>
> Thanks,
>
> Michael
>
> On 9/30/19, 2:29 PM, "ocket 8888" <oc...@gmail.com> wrote:
>
> Let me start by saying I'm all for a linter for the Go codebase(s).
>
> This collection of linters seems to have a lot of redundancy, and be
> built
> with using their specific CI platform in mind. In just the default
> linters
> it lists, it has four or five different linters to check for unused
> code,
> something that is performed already by the compiler (type checking),
> and
> both "govet" and something that describes itself as "govet on
> steroids". So
> I wonder if maybe we want to pick something smaller and less redundant.
>
> On Mon, Sep 30, 2019 at 8:28 AM Hoppal, Michael <
> Michael_Hoppal@comcast.com>
> 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://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://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
> >
> >
> >
>
>
>