Re: [Puppet-dev] Re: RFC: Static Analysis of the Puppet project

2014-08-10 Thread Kylo Ginsberg
On Sat, Aug 9, 2014 at 9:45 AM, Henrik Lindberg 
henrik.lindb...@cloudsmith.com wrote:

 On 2014-09-08 24:13, Andy Parker wrote:

 On Fri, Aug 8, 2014 at 3:11 PM, Rahul Gopinath ra...@puppetlabs.com
 mailto:ra...@puppetlabs.com wrote:

 While correcting AndOr, I come across methods calls such as  `if
 value.is_a? FixNum or value.is_a? Integer`

 How should we parenthesize it? is it `(value.is_a? FixNum) ||
 (value.is_a? Integer)`  or should it be `value.is_a?(FixNum) ||
 value.is_a?(Integer)` ?


 value.is_a?(FixNum)

 Parens around the method arguments are the clearest fix and we should
 put them in everywhere IMO :)


 +1, except in internal DSL like logic where it reduces readability


I definitely agree on the parentheses example above.

Wrt requiring parens around method arguments *everywhere*, I have to admit
I've wanted that at times when reading puppet code, but I also suspect it
would be very high touch and perhaps controversial.

And it sounds like we don't have consensus on that one. Henrik, can you
give an example or two where it would reduce readability? That might help
guide the discussion a bit.


   So I think we should turn on the `Style/Not` cop
  
   Either way, I'm a +1 for adding in this rule too.
  
   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).
  


 +1


Sounds like we have agreement on Style/Not and Style/SpaceAfterNot. Shout
now if you disagree!

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/CALsUZFF2nFyOpvTB5Ap1mEvZ_NaC2y-VN00Pic_ZdKt%2B0_rh0g%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


Re: [Puppet-dev] Re: RFC: Static Analysis of the Puppet project

2014-08-10 Thread Rahul Gopinath
Here is a related issue,

I notice that there are different conventions for spacing followed
when using method calls with parenthesis,
including `method(a,b)`, `method( a, b )` and `method( a,b )`. Does
the list have a preference for one of the styles when I add
parenthesis (to those methods that are not DSL like)?

I think our choices are
`mymethod(a,b)`   -- currently supported by rubocop
`mymethod( a, b )`  -- another common style

or any other...


On Sun, Aug 10, 2014 at 5:01 PM, Kylo Ginsberg k...@puppetlabs.com wrote:
 On Sat, Aug 9, 2014 at 9:45 AM, Henrik Lindberg
 henrik.lindb...@cloudsmith.com wrote:

 On 2014-09-08 24:13, Andy Parker wrote:

 On Fri, Aug 8, 2014 at 3:11 PM, Rahul Gopinath ra...@puppetlabs.com
 mailto:ra...@puppetlabs.com wrote:

 While correcting AndOr, I come across methods calls such as  `if
 value.is_a? FixNum or value.is_a? Integer`

 How should we parenthesize it? is it `(value.is_a? FixNum) ||
 (value.is_a? Integer)`  or should it be `value.is_a?(FixNum) ||
 value.is_a?(Integer)` ?


 value.is_a?(FixNum)

 Parens around the method arguments are the clearest fix and we should
 put them in everywhere IMO :)


 +1, except in internal DSL like logic where it reduces readability


 I definitely agree on the parentheses example above.

 Wrt requiring parens around method arguments *everywhere*, I have to admit
 I've wanted that at times when reading puppet code, but I also suspect it
 would be very high touch and perhaps controversial.

 And it sounds like we don't have consensus on that one. Henrik, can you give
 an example or two where it would reduce readability? That might help guide
 the discussion a bit.


   So I think we should turn on the `Style/Not` cop
  

   Either way, I'm a +1 for adding in this rule too.
  
   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).
  


 +1


 Sounds like we have agreement on Style/Not and Style/SpaceAfterNot. Shout
 now if you disagree!

 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/CALsUZFF2nFyOpvTB5Ap1mEvZ_NaC2y-VN00Pic_ZdKt%2B0_rh0g%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%2BemFfxPdki%2BAFHHpSdCC%3D5z6DScOswsOZpw6aq%3Dqdsz%3Dh%3Dt8Q%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


Re: [Puppet-dev] Re: Feedback on the behavior of +=, -=, +, and -

2014-08-10 Thread Trevor Vaughan
Yeah, I know that it doesn't actually mutate. But it *feels* like it does,
which is the issue.

Trevor


On Sat, Aug 9, 2014 at 4:37 PM, Henrik Lindberg 
henrik.lindb...@cloudsmith.com wrote:

 On 2014-09-08 21:12, Trevor Vaughan wrote:

 I tend to avoid += since it's a magic edge case in variable scope.

 If all of my code takes the strict view that variables are immutable, I
 am much less likely to confuse anyone who reads my code later.


 Just to be clear, the += does *not* mutate, it creates a new variable that
 shadows the original in an outer scope.


  This does end up with stupid code like $l_var = $var + ['foo'], but that
 can't be helped while preserving clarity.

 Now, I would LOVE the ability to manipulate all variables within scope.
 That would be very nice and then I would vote for keeping += around.

 That would allow for things like:

 if empty($foo) {
$foo = 'bar'
 }

 Instead of:

 if empty($foo) {
$l_foo = 'bar'
 }
 else {
$l_foo = $foo
 }

 Which is irritating.

 Trevor


 - henrik
 --

 Visit my Blog Puppet on the Edge
 http://puppet-on-the-edge.blogspot.se/


 --
 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/ls60q9%24s0d%241%40ger.gmane.org.

 For more options, visit https://groups.google.com/d/optout.




-- 
Trevor Vaughan
Vice President, Onyx Point, Inc
(410) 541-6699
tvaug...@onyxpoint.com

-- This account not approved for unencrypted proprietary information --

-- 
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/CANs%2BFoWOPrVQnMWchtB1d6-B1xj6aEBY9Ww%2Bw3F-GH8dQ%2B00JQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


Re: [Puppet-dev] Re: Feedback on the behavior of +=, -=, +, and -

2014-08-10 Thread Reid Vandewiele
On Sunday, August 10, 2014 7:11:11 PM UTC-7, Trevor Vaughan wrote:

 Yeah, I know that it doesn't actually mutate. But it *feels* like it does, 
 which is the issue.

 Trevor


For this reason I would advocate omission of += and -= from the language.

The problem is not that the behavior is inconsistent or that it breaks any 
-rules-, per se. The problem is that the behavior is non-intuitive and not 
just in a difficult-to-guess-at way, but in a can-directly-confuse-users 
way. Yes, $fqdn is potentially different from $::fqdn but if we're trying 
to guide people into a mindset of variables are immutable we should not 
muddy the waters with syntax that looks contradictory to that paradigm - 
especially if all it gains us is saving a few characters being typed.

I believe this constitutes a compelling design reason to remove += and -=.

~Reid

-- 
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/4a0b0503-dd0a-408f-b856-afe91374a924%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.