On Thu, Aug 7, 2014 at 6:09 PM, Rahul Gopinath <[email protected]> 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 <[email protected]> wrote: > > On Wed, Aug 6, 2014 at 5:39 PM, Kylo Ginsberg <[email protected]> > wrote: > >> > >> On Tue, Aug 5, 2014 at 3:50 PM, rahul <[email protected]> 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 <[email protected]> > >>>> 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 [email protected]. > >>> 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 > >> [email protected] > >> > >> 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 > > [email protected] > > > > 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 [email protected]. > > 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 [email protected]. > 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 [email protected] 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 [email protected]. 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.
