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 <http://www.puppetconf.com/>, September 20-24 in San Francisco* *Register by July 31st to take advantage of the Early Bird discount <https://puppetconf2014.eventbrite.com/?discount=EarlyBird> **—**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.
