For the ease of management, I would like to split that into two PR, one dealing with the original Lint/* and a second one dealing with Style/AndOr since the number of violations of Style/AndOr is really large, and it may be better to tackle it separately.
On Thu, Jul 17, 2014 at 9:23 AM, Adrien Thebo <adr...@puppetlabs.com> wrote: > I agree, I think it's better to get this system turned on and catching > issues now and we can add more cops later as the code base improves. > > > On Thu, Jul 17, 2014 at 7:11 AM, Kylo Ginsberg <k...@puppetlabs.com> wrote: >> >> On Wed, Jul 16, 2014 at 10:34 AM, Rahul Gopinath <ra...@puppetlabs.com> >> wrote: >>> >>> Thanks, I have updated the PR >>> >>> https://github.com/vrthra/puppet/commit/f4f9fc4e333b2e53d63ca4b8e00d02a4f2bd47f8 >>> >>> On Wed, Jul 16, 2014 at 10:03 AM, Erik Dalén >>> <erik.gustav.da...@gmail.com> wrote: >>> > right, the generated files are: >>> > lib/puppet/parser/parser.rb >>> > lib/puppet/pops/parser/eparser.rb >>> > lib/puppet/external/nagios/parser.rb >>> > >>> > They are generated from those .ry and .ra files. >>> > >>> > >>> > >>> > On 16 July 2014 18:12, Rahul Gopinath <ra...@puppetlabs.com> wrote: >>> >> >>> >> I see only *.ra|*.ry files (no grammar.rb) >>> >> >>> >> | find . | grep grammar >>> >> ./lib/puppet/external/nagios/grammar.ry >>> >> ./lib/puppet/parser/grammar.ra >>> >> ./lib/puppet/pops/parser/egrammar.ra >>> >> >>> >> We are currently limiting the scanning to *.rb files >>> >> >>> >> On Wed, Jul 16, 2014 at 12:21 AM, Erik Dalén >>> >> <erik.gustav.da...@gmail.com> wrote: >>> >> > Don't know how many they are causing, but you should probably >>> >> > exclude >>> >> > the >>> >> > generated grammar.rb and egrammar.rb files. The PR should be updated >>> >> > to >>> >> > do >>> >> > this as well. >>> >> > >>> >> > >>> >> > On 15 July 2014 19:46, rahul <ra...@puppetlabs.com> wrote: >>> >> >> >>> >> >> The total number of offenses on enabling all cops is 38303, of >>> >> >> which >>> >> >> 8769 >>> >> >> are in lib/puppet/pops >>> >> >> Not all the cops may be useful, and a few of them are >>> >> >> controversial. >>> >> >> >>> >> >> >>> >> >> On Monday, July 14, 2014 11:56:16 AM UTC-7, Brian LaMetterey wrote: >>> >> >>> >>> >> >>> Keep in mind that we can always take a layered approach. Could >>> >> >>> hire a >>> >> >>> small number of cops, then add more as our crime rate decreases. >>> >> >>> >>> >> >>> Have we done an initial run to see how much crime we have? Is it >>> >> >>> a >>> >> >>> daunting amount? >>> >> >>> >>> >> >>> >>> >> >>> On Mon, Jul 14, 2014 at 11:07 AM, Rob Reynolds >>> >> >>> <r...@puppetlabs.com> >>> >> >>> wrote: >>> >> >>>> >>> >> >>>> >>> >> >>>> >>> >> >>>> On Mon, Jul 14, 2014 at 12:56 PM, Kylo Ginsberg >>> >> >>>> <ky...@puppetlabs.com> >>> >> >>>> wrote: >>> >> >>>>> >>> >> >>>>> HI all, >>> >> >>>>> >>> >> >>>>> We'd like to start using static analysis against the puppet code >>> >> >>>>> base >>> >> >>>>> both to catch certain classes of coding errors and to enforce >>> >> >>>>> best >>> >> >>>>> coding >>> >> >>>>> practices. Those are laudable goals of course, but there is >>> >> >>>>> plenty >>> >> >>>>> of room >>> >> >>>>> for opinions on what qualifies. This email is a request to >>> >> >>>>> solicit >>> >> >>>>> some >>> >> >>>>> opinions :) >>> >> >>>>> >>> >> >>>>> To kick the discussion off: at this point, we're leaning toward >>> >> >>>>> using >>> >> >>>>> rubocop for static analysis, identifying a set of checkers >>> >> >>>>> ('cops' >>> >> >>>>> in >>> >> >>>>> rubocop lingo) and then setting up some CI integration, either >>> >> >>>>> in >>> >> >>>>> travis-ci >>> >> >>>>> or houndci, to enforce those cops against PRs. >>> >> >>>>> >>> >> >>>>> Rahul Gopinath has put together a PR with an initial proposal of >>> >> >>>>> 'cops' >>> >> >>>>> we might use: >>> >> >>>>> >>> >> >>>>> https://github.com/puppetlabs/puppet/pull/2855 >>> >> >>>>> >>> >> >>>>> There's some initial discussion in that PR but the tldr of the >>> >> >>>>> proposal >>> >> >>>>> is to enable these cops: >>> >> >>>>> >>> >> >>>>> Lint/UnreachableCode >>> >> >>>>> Lint/ConditionPosition >>> >> >>>>> Lint/UselessComparison >>> >> >>>>> Lint/LiteralInterpolation >>> >> >>>>> Lint/ElseLayout >>> >> >>>>> >>> >> >>>>> and then there's been some discussion on the PR around these two >>> >> >>>>> cops: >>> >> >>>>> >>> >> >>>>> Style/AndOr >>> >> >>>>> Lint/AssignmentInCondition >>> >> >>>>> >>> >> >>>>> Each of those two checks catch coding patterns which both are a >>> >> >>>>> source >>> >> >>>>> of some bugs and, at the same time are idiomatic in certain >>> >> >>>>> cases. >>> >> >>>>> So >>> >> >>>>> there's room for discussion on those two. >>> >> >>>>> >>> >> >>>>> And then there are a *bunch* more cops for a variety of >>> >> >>>>> style/lint >>> >> >>>>> checks which we could consider enabling in addition to the >>> >> >>>>> above. >>> >> >>>>> There's >>> >> >>>>> some documentation of the various cops in the rubocop yaml files >>> >> >>>>> at: >>> >> >>>>> >>> >> >>>>> https://github.com/bbatsov/rubocop/tree/master/config >>> >> >>>>> >>> >> >>>>> So, thoughts? >>> >> >>>>> >>> >> >>>>> Kylo >>> >> >>>>> >>> >> >>>>> -- >>> >> >>>>> Kylo Ginsberg >>> >> >>>>> ky...@puppetlabs.com >>> >> >>>>> >>> >> >>>>> Join us at PuppetConf 2014, September 20-24 in San Francisco >>> >> >>>>> Register by July 31st to take advantage of the Early Bird >>> >> >>>>> discount >>> >> >>>>> —save $249! >>> >> >>>>> >>> >> >>>>> -- >>> >> >>>>> You received this message because you are subscribed to the >>> >> >>>>> Google >>> >> >>>>> Groups "Puppet Developers" group. >>> >> >>>>> To unsubscribe from this group and stop receiving emails from >>> >> >>>>> it, >>> >> >>>>> send >>> >> >>>>> an email to puppet-dev+...@googlegroups.com. >>> >> >>>>> >>> >> >>>>> To view this discussion on the web visit >>> >> >>>>> >>> >> >>>>> >>> >> >>>>> https://groups.google.com/d/msgid/puppet-dev/CALsUZFHmU%2B8aAHLNV3nu5HK98d4%2BEw0Ez-GBJZHpTD7gddSSJA%40mail.gmail.com. >>> >> >>>>> For more options, visit https://groups.google.com/d/optout. >>> >> >>>> >>> >> >>>> >>> >> >>>> >>> >> >>>> I think it would greatly increase the quality of contributions if >>> >> >>>> the >>> >> >>>> "cops" started catching things and failing the PR builds. Being >>> >> >>>> picky >>> >> >>>> with >>> >> >>>> what we start evaluating I think is the right call and what Andy >>> >> >>>> and >>> >> >>>> Rahul >>> >> >>>> were already working out. >>> >> >>>> >>> >> >>>> >>> >> >>>> -- >>> >> >>>> Rob Reynolds >>> >> >>>> Developer, Puppet Labs >>> >> >>>> >>> >> >>>> Join us at PuppetConf 2014, September 20-24 in San Francisco >>> >> >>>> Register by July 31st to take advantage of the Early Bird >>> >> >>>> discount >>> >> >>>> —save >>> >> >>>> $249! >>> >> >>>> >>> >> >>>> -- >>> >> >>>> You received this message because you are subscribed to the >>> >> >>>> Google >>> >> >>>> Groups "Puppet Developers" group. >>> >> >>>> To unsubscribe from this group and stop receiving emails from it, >>> >> >>>> send >>> >> >>>> an email to puppet-dev+...@googlegroups.com. >>> >> >>>> To view this discussion on the web visit >>> >> >>>> >>> >> >>>> >>> >> >>>> https://groups.google.com/d/msgid/puppet-dev/CAMJiBK4ZzCG_5Noa-3ctfcmgHCArXri6wqXUnbypeQ%3DK%3Dnxz_A%40mail.gmail.com. >>> >> >>>> >>> >> >>>> For more options, visit https://groups.google.com/d/optout. >>> >> >>> >>> >> >>> >>> >> >>> >>> >> >>> >>> >> >>> -- >>> >> >>> Join us at PuppetConf 2014, September 22-24 in San Francisco - >>> >> >>> http://puppetconf.com >>> >> >>> Register by July 31st to take advantage of the Early Bird discount >>> >> >>> —save >>> >> >>> $249! >>> >> >> >>> >> >> -- >>> >> >> You received this message because you are subscribed to the Google >>> >> >> Groups >>> >> >> "Puppet Developers" group. >>> >> >> To unsubscribe from this group and stop receiving emails from it, >>> >> >> send >>> >> >> an >>> >> >> email to puppet-dev+unsubscr...@googlegroups.com. >>> >> >> To view this discussion on the web visit >>> >> >> >>> >> >> >>> >> >> https://groups.google.com/d/msgid/puppet-dev/77907278-9756-4ec4-a7fe-4d165a3cf9db%40googlegroups.com. >>> >> >> >>> >> >> For more options, visit https://groups.google.com/d/optout. >>> >> > >>> >> > >>> >> > >>> >> > >>> >> > -- >>> >> > Erik Dalén >>> >> > >>> >> > -- >>> >> > You received this message because you are subscribed to the Google >>> >> > Groups >>> >> > "Puppet Developers" group. >>> >> > To unsubscribe from this group and stop receiving emails from it, >>> >> > send >>> >> > an >>> >> > email to puppet-dev+unsubscr...@googlegroups.com. >>> >> > To view this discussion on the web visit >>> >> > >>> >> > >>> >> > https://groups.google.com/d/msgid/puppet-dev/CAAAzDLfzpN1wMivHNYsg%2BwWqgd5qG7D%3D5avapBDFvN214HPNSQ%40mail.gmail.com. >>> >> > >>> >> > For more options, visit https://groups.google.com/d/optout. >>> >> >>> >> -- >>> >> You received this message because you are subscribed to the Google >>> >> Groups >>> >> "Puppet Developers" group. >>> >> To unsubscribe from this group and stop receiving emails from it, send >>> >> an >>> >> email to puppet-dev+unsubscr...@googlegroups.com. >>> >> To view this discussion on the web visit >>> >> >>> >> https://groups.google.com/d/msgid/puppet-dev/CA%2BemFfzOtUwAp7otOUZ-oo0PcSbKSf8BRLFkGhGgu6eBUubj6A%40mail.gmail.com. >>> >> >>> >> For more options, visit https://groups.google.com/d/optout. >>> > >>> > >>> > >>> > >>> > -- >>> > Erik Dalén >>> > >> >> >> To move this forward, I propose we keep this first pass on the modest side >> and stick to the cops that have mostly nods above: >> >> Lint/UnreachableCode >> Lint/ConditionPosition >> Lint/UselessComparison >> Lint/LiteralInterpolation >> Lint/ElseLayout >> Style/AndOr >> >> This will allow us to focus on integrating rubocop into the CI pipeline >> (jenkins and travis/hound) and sorting out any issues there. >> >> Once we have something in place and get a feel for how it works, we can of >> course have follow-on PRs for other cops discussed above like >> assignment-in-conditionals or formatting, etc, and those can be discussed as >> with any other PR. >> >> -- >> Kylo Ginsberg >> k...@puppetlabs.com >> >> Join us at PuppetConf 2014, September 20-24 in San Francisco >> Register by July 31st to take advantage of the Early Bird discount —save >> $249! >> >> -- >> You received this message because you are subscribed to the Google Groups >> "Puppet Developers" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to puppet-dev+unsubscr...@googlegroups.com. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/puppet-dev/CALsUZFG5Uf3PUKnrVCH33RzaXEA2UZX0YtPzhyiWnxZKih3uwA%40mail.gmail.com. >> >> For more options, visit https://groups.google.com/d/optout. > > > > > -- > Adrien Thebo | Puppet Labs > > -- > You received this message because you are subscribed to the Google Groups > "Puppet Developers" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to puppet-dev+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/puppet-dev/CALVJ9SJ5mQc6u9wRjFNnsURBAfbJU%3D9dc-owcNYNwkwVd8u23Q%40mail.gmail.com. > > For more options, visit https://groups.google.com/d/optout. -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to puppet-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/CA%2BemFfyOH0XPsBfU6%2BHgFOOqJ-bV1VASzKYJ8iJYyOaVUnKLFg%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
