On Thu, Aug 7, 2014 at 6:09 PM, Rahul Gopinath <ra...@puppetlabs.com> wrote:
> While hacking rubocop, I found that I can get it to autocorrect even > more `Style/AndOr` violations if I replace the use of the `not` > keyword with `!` first. The Rubocop is able to do the necessary > changes automatically. So I think we should turn on the `Style/Not` > cop also for our code for three reasons > 1. It is essentially free, with rubocop able to supply the entire set > of corrections in its autocorrect mode > 2. It is consistent without goal of avoiding keywords with confusing > precedence. > I actually thought that not would have gone hand in hand with and/or. Doesn't it have the same precedence problems as and/or? Either way, I'm a +1 for adding in this rule too. > 3. Rubocop autocorrect is the best we can hope for, since it ensures > that the AST resulting from the changes are same, and hence the > semantics of the new and old fragments are same. Hence we avoid bugs > that cold go undetected otherwise. > I also propose to turn on the `Style/SpaceAfterNot` which disallows > space after the operator `!` so that usage such as `! mymethod` is > flagged in favor or `!mymethod` > > No space after ! is my preferred style. And I'll just stop any style wars and say it is the preferred style of the codebase (I am channelling the puppet code gods). > Are there any objections to both these? > > If you would like to see what changes these two require, see my PR at > https://github.com/puppetlabs/puppet/pull/2944 > > > On Thu, Aug 7, 2014 at 9:39 AM, Kylo Ginsberg <k...@puppetlabs.com> wrote: > > On Wed, Aug 6, 2014 at 5:39 PM, Kylo Ginsberg <k...@puppetlabs.com> > wrote: > >> > >> On Tue, Aug 5, 2014 at 3:50 PM, rahul <ra...@puppetlabs.com> wrote: > >>> > >>> So to summarize, this is our plan for Rubocop: > >>> > >>> - We propose to enable AndOr cop in small chunks, giving preference to > >>> those files/directories that are heavily in development. > >>> - For AndOr, the conclusion seems to be to avoid keywords completely, > and > >>> ensure that the instances where they are used are changed do not hurt > >>> readability. > >>> - As a prototype, we have turned on AndOr on lib/pops directory PR 2892 > >> > >> > >> Also a heads-up that for pull requests: > >> > >> 1) a week or so ago, we added a travis job that fails if any of the > >> .rubocop.yml enabled cops report anything (these are just the cops that > were > >> uncontroversial at the beginning of this thread) > >> > >> 2) just now, I turned on houndci.com which will comment on pull > requests > >> based on the same configuration > >> > >> Note that hound *can* be configured with a separate config file of its > >> own, but we don't have one, so it falls back to the .rubocop.yml. If we > >> wanted to have a set of cops which triggered comments on the PRs, but > didn't > >> figure travis fails, we could get that by having a separate > houndci.yml. Not > >> sure what I think of that, but just putting the idea out there. > > > > > > Actually houndci doesn't seem to be respecting our .rubocop.yml so I > turned > > it off for now. > > > > Kylo > > > >> > >> > >> Kylo > >> > >>> > >>> > >>> On Tuesday, July 29, 2014 11:00:46 PM UTC-7, Kylo Ginsberg wrote: > >>>> > >>>> On Tue, Jul 29, 2014 at 4:42 PM, Andy Parker <an...@puppetlabs.com> > >>>> wrote: > >>>>> > >>>>> Right now the PRs are doing a mechanical transformation to remove a > >>>>> keyword that we don't want to use. What is missing is that it isn't > >>>>> transforming the code into what later changes to that code should > preserve. > >>>>> Or put another way, if we got a PR that contained new code that > looked like > >>>>> that we would reject it, I think. It passes the test of not using > disallowed > >>>>> operators, but it doesn't pass the test of being written in a form > that a > >>>>> reader would expect. > >>>> > >>>> > >>>> I agree that the purely mechanical transformation applied to the > >>>> genuinely flow control cases introduces constructs that would slow me > down > >>>> as a code reader (and that I'd be very unlikely to write). > >>>> > >>>> So are there objections to converting such cases to use 'if', etc? > >>>> Personally I'd find that clearer and easier to read. And it would > still > >>>> allow us to eliminate the and/or keywords which we've identified as > the > >>>> source of some bugs/confusion. > >>>> > >>>> Kylo > >>> > >>> -- > >>> 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/ab872dad-c258-4e09-81b3-8c13f17bc968%40googlegroups.com > . > >>> > >>> For more options, visit https://groups.google.com/d/optout. > >> > >> > >> > >> > >> -- > >> Kylo Ginsberg > >> k...@puppetlabs.com > >> > >> Join us at PuppetConf 2014, September 20-24 in San Francisco > >> Register by September 8th to take advantage of the Final Countdown —save > >> $149! > > > > > > > > > > -- > > Kylo Ginsberg > > k...@puppetlabs.com > > > > Join us at PuppetConf 2014, September 20-24 in San Francisco > > Register by September 8th to take advantage of the Final Countdown —save > > $149! > > > > -- > > 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/CALsUZFF4NF%3D6hoA3DUb6NturZ1KHLR2Y4bNiwoYiVrVse3fzgg%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%2BemFfz56%3DNTedoRVbM32FVCZpnKov7BoSwewTpJR9rc%2BR2QRA%40mail.gmail.com > . > For more options, visit https://groups.google.com/d/optout. > -- Andrew Parker a...@puppetlabs.com Freenode: zaphod42 Twitter: @aparker42 Software Developer *Join us at PuppetConf 2014 <http://www.puppetconf.com/>, September 22-24 in San Francisco* *Register by May 30th to take advantage of the Early Adopter discount <http://links.puppetlabs.com/puppetconf-early-adopter> **—**save $349!* -- 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/CANhgQXtd%3DZ0Qr9eqJOopqYFP4ER_xnCAcgKomm8wPNWy42zmsQ%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
