On Thu, Jul 17, 2014 at 10:06 AM, Rahul Gopinath <[email protected]> wrote:
> 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. > +1 > > 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. > -- 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/CALsUZFG_gtNQk5qAFES4Ku09nd70iTCLZ9LDgsT_ZFQLh5h%3DeQ%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
