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 <[email protected]> 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 <[email protected]> wrote: >> >> On Wed, Jul 16, 2014 at 10:34 AM, Rahul Gopinath <[email protected]> >> 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 >>> <[email protected]> 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 <[email protected]> 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 >>> >> <[email protected]> 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 <[email protected]> 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 >>> >> >>> <[email protected]> >>> >> >>> wrote: >>> >> >>>> >>> >> >>>> >>> >> >>>> >>> >> >>>> On Mon, Jul 14, 2014 at 12:56 PM, Kylo Ginsberg >>> >> >>>> <[email protected]> >>> >> >>>> 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 >>> >> >>>>> [email protected] >>> >> >>>>> >>> >> >>>>> 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 [email protected]. >>> >> >>>>> >>> >> >>>>> 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 [email protected]. >>> >> >>>> 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 [email protected]. >>> >> >> 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 [email protected]. >>> >> > 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 [email protected]. >>> >> 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 >> [email protected] >> >> 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 [email protected]. >> 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 [email protected]. > 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 [email protected]. 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.
