I have a PR regarding orchestration [1] (orchestration methods returning false accidentally
even though there was no error for funny reasons such as changing logging levels or
puppet ca proxy not being assigned to a puppet ca). I'm proposing turning off the ReduntantReturn style
as there are places (such as here), where the return can emphasize that the return
value is really significant, and the standalone boolean at the end of methods just doesn't look good.
I'm suggesting turning this off globally, as I believe (and maybe I'm alone here) that there
are situations where the explicit return can be justified (to emphasize the return value is significant
where it might look like it isn't) and I don't want for people to feel guilty about that
You're not alone. I prefer to be able to call return explicitly. I'm missing
return in following example
def answer(q)
if q == 'question'
return 'answer'
end
42
end
I don't see any negatives. Based on PR discussion, I don't understand why it
would require extra effort from reviewer. It's fine when someone uses it, just
ignore it.
So +1 from me for disabling globally.
···
On Tuesday 29 of March 2016 10:00:00 Ivan Necas wrote:
> Hi,
>
> I have a PR regarding orchestration [1] (orchestration methods returning
> false accidentally even though there was no error for funny reasons such as
> changing logging levels or puppet ca proxy not being assigned to a puppet
> ca). I'm proposing turning off the ReduntantReturn style as there are
> places (such as here), where the return can emphasize that the return value
> is really significant, and the standalone boolean at the end of methods
> just doesn't look good.
>
> I'm suggesting turning this off globally, as I believe (and maybe I'm alone
> here) that there are situations where the explicit return can be justified
> (to emphasize the return value is significant where it might look like it
> isn't) and I don't want for people to feel guilty about that :)
>
> Thoughts?
> > Hi,
> >
> > I have a PR regarding orchestration [1] (orchestration methods returning
> > false accidentally even though there was no error for funny reasons such as
> > changing logging levels or puppet ca proxy not being assigned to a puppet
> > ca). I'm proposing turning off the ReduntantReturn style as there are
> > places (such as here), where the return can emphasize that the return value
> > is really significant, and the standalone boolean at the end of methods
> > just doesn't look good.
Doesn't look good is very subjective. It does look good to me? Calling return in ruby other than in a guard clause (shortcut exit to a method)
isn't standard
> >
> > I'm suggesting turning this off globally, as I believe (and maybe I'm alone
> > here) that there are situations where the explicit return can be justified
> > (to emphasize the return value is significant where it might look like it
> > isn't) and I don't want for people to feel guilty about that
> >
> > Thoughts?
>
> You're not alone. I prefer to be able to call return explicitly. I'm missing
> return in following example
>
> def answer(q)
> if q == 'question'
> return 'answer'
> end
> 42
> end
I tend to abide by the ruby style guide - that method would be OK (and
idiomatic) like
def answer(q)
return 'answer' if q == 'question'
42
end
Adding the option of having return wherever just makes the code base
less coherent. Especially if it has hidden meanings like 'hey this
return means something'.
···
On 03/30, Marek Hulán wrote:
> On Tuesday 29 of March 2016 10:00:00 Ivan Necas wrote:
All in all it’s mostly a style convention, which is why Rubocop exists
and 1000s of rubyists decide to use it, to stop these discussions from
happening and standardize on one style. All major style guides use it
(fallacy of appeal to authority, I know), for a reason - so that code
looks like it was written by one person and it’s more readable.
The whole point for that cop to exist is to stop people from discussing
about this, and spend that time writing (coherent, homogeneous) code.
I don’t see any negatives. Based on PR discussion, I don’t understand why it
would require extra effort from reviewer. It’s fine when someone uses it, just
ignore it.
I tend to agree with Daniel. Personally I find it confusing to see a method
that ends in a return statement as I’ve come to associate return with a
change in the flow of control. I also worry that if we disable the cop,
we’re going to see a mixture of methods that either do or do not have
returns.
If someone really wants to use the return keyword to clarify something, I
am fine with just disabling the cop on a single line or file. But +1 to
keeping the cop on globally from me.
David
···
On Wed, Mar 30, 2016 at 6:37 AM, Daniel Lobato Garcia wrote:
On 03/30, Marek Hulán wrote:
On Tuesday 29 of March 2016 10:00:00 Ivan Necas wrote:
Hi,
I have a PR regarding orchestration [1] (orchestration methods
returning
false accidentally even though there was no error for funny reasons
such as
changing logging levels or puppet ca proxy not being assigned to a
puppet
ca). I’m proposing turning off the ReduntantReturn style as there are
places (such as here), where the return can emphasize that the return
value
is really significant, and the standalone boolean at the end of methods
just doesn’t look good.
Doesn’t look good is very subjective. It does look good to me? Calling return in ruby other than in a guard clause (shortcut exit to a method)
isn’t standard
I’m suggesting turning this off globally, as I believe (and maybe I’m
alone
here) that there are situations where the explicit return can be
justified
(to emphasize the return value is significant where it might look like
it
isn’t) and I don’t want for people to feel guilty about that
Thoughts?
You’re not alone. I prefer to be able to call return explicitly. I’m
missing
return in following example
def answer(q)
if q == 'question’
return 'answer’
end
42
end
I tend to abide by the ruby style guide - that method would be OK (and
idiomatic) like
def answer(q)
return ‘answer’ if q == 'question’
42
end
Adding the option of having return wherever just makes the code base
less coherent. Especially if it has hidden meanings like ‘hey this
return means something’.
All in all it’s mostly a style convention, which is why Rubocop exists
and 1000s of rubyists decide to use it, to stop these discussions from
happening and standardize on one style. All major style guides use it
(fallacy of appeal to authority, I know), for a reason - so that code
looks like it was written by one person and it’s more readable.
The whole point for that cop to exist is to stop people from discussing
about this, and spend that time writing (coherent, homogeneous) code.
I don’t see any negatives. Based on PR discussion, I don’t understand
why it
would require extra effort from reviewer. It’s fine when someone uses
it, just
ignore it.
Obviously that was artificial example and the whole method could be one
expression with ternary operator but that wasn't the point. If the logic above
42 was more complex, the 42 itself seems easier to miss. And (subjectively)
doesn't look good
Anyway I'd be happy to hear more (subjective) opinions, then choose the most
favorite style and use it.
···
On Wednesday 30 of March 2016 12:37:16 Daniel Lobato Garcia wrote:
> On 03/30, Marek Hulán wrote:
> > On Tuesday 29 of March 2016 10:00:00 Ivan Necas wrote:
> > > Hi,
> > >
> > > I have a PR regarding orchestration [1] (orchestration methods returning
> > > false accidentally even though there was no error for funny reasons such
> > > as
> > > changing logging levels or puppet ca proxy not being assigned to a
> > > puppet
> > > ca). I'm proposing turning off the ReduntantReturn style as there are
> > > places (such as here), where the return can emphasize that the return
> > > value
> > > is really significant, and the standalone boolean at the end of methods
> > > just doesn't look good.
>
> Doesn't look good is very subjective. It does look good to me? Calling
> `return` in ruby other than in a guard clause (shortcut exit to a method)
> isn't standard
>
> > > I'm suggesting turning this off globally, as I believe (and maybe I'm
> > > alone
> > > here) that there are situations where the explicit return can be
> > > justified
> > > (to emphasize the return value is significant where it might look like
> > > it
> > > isn't) and I don't want for people to feel guilty about that :)
> > >
> > > Thoughts?
> >
> > You're not alone. I prefer to be able to call return explicitly. I'm
> > missing return in following example
> >
> > def answer(q)
> >
> > if q == 'question'
> >
> > return 'answer'
> >
> > end
> > 42
> >
> > end
>
> I tend to abide by the ruby style guide - that method would be OK (and
> idiomatic) like
>
> def answer(q)
> return 'answer' if q == 'question'
> 42
> end
–
Marek
Adding the option of having return wherever just makes the code base
less coherent. Especially if it has hidden meanings like ‘hey this
return means something’.
All in all it’s mostly a style convention, which is why Rubocop exists
and 1000s of rubyists decide to use it, to stop these discussions from
happening and standardize on one style. All major style guides use it
(fallacy of appeal to authority, I know), for a reason - so that code
looks like it was written by one person and it’s more readable.
The whole point for that cop to exist is to stop people from discussing
about this, and spend that time writing (coherent, homogeneous) code.
I don’t see any negatives. Based on PR discussion, I don’t understand why
it would require extra effort from reviewer. It’s fine when someone uses
it, just ignore it.
I agree that it would be nicer, for the procedure-like method (as we do have in orchestration), to not expect the
return value, but rather have other mechanism to indicate a failure but I'm not sure we can change that without breaking some backward compatibilities.
It seems we have 3:2 in favor of keeping the cop on. I leaving this thread opened until Monday (including). If no other folks express their preference, I will take it as they are happy about how it's done right now, which is something I'm not very happy about, but we can't make everyone happy, when it comes to enabling/disabling cops. I still think the discussion about enabled cops in general is important, as I don't think all the cops make the world a better place (as in real-world, there are good and bad cops:).
– Ivan
···
----- Original Message -----
> On Wednesday 30 of March 2016 12:37:16 Daniel Lobato Garcia wrote:
> > On 03/30, Marek Hulán wrote:
> > > On Tuesday 29 of March 2016 10:00:00 Ivan Necas wrote:
> > > > Hi,
> > > >
> > > > I have a PR regarding orchestration [1] (orchestration methods
> > > > returning
> > > > false accidentally even though there was no error for funny reasons
> > > > such
> > > > as
> > > > changing logging levels or puppet ca proxy not being assigned to a
> > > > puppet
> > > > ca). I'm proposing turning off the ReduntantReturn style as there are
> > > > places (such as here), where the return can emphasize that the return
> > > > value
> > > > is really significant, and the standalone boolean at the end of methods
> > > > just doesn't look good.
> >
> > Doesn't look good is very subjective. It does look good to me? Calling
> > `return` in ruby other than in a guard clause (shortcut exit to a method)
> > isn't standard
> >
> > > > I'm suggesting turning this off globally, as I believe (and maybe I'm
> > > > alone
> > > > here) that there are situations where the explicit return can be
> > > > justified
> > > > (to emphasize the return value is significant where it might look like
> > > > it
> > > > isn't) and I don't want for people to feel guilty about that :)
> > > >
> > > > Thoughts?
> > >
> > > You're not alone. I prefer to be able to call return explicitly. I'm
> > > missing return in following example
> > >
> > > def answer(q)
> > >
> > > if q == 'question'
> > >
> > > return 'answer'
> > >
> > > end
> > > 42
> > >
> > > end
> >
> > I tend to abide by the ruby style guide - that method would be OK (and
> > idiomatic) like
> >
> > def answer(q)
> > return 'answer' if q == 'question'
> > 42
> > end
>
> Obviously that was artificial example and the whole method could be one
> expression with ternary operator but that wasn't the point. If the logic
> above
> 42 was more complex, the 42 itself seems easier to miss. And (subjectively)
> doesn't look good :-)
>
> Anyway I'd be happy to hear more (subjective) opinions, then choose the most
> favorite style and use it.
>
> --
> Marek
>
> >
> > Adding the option of having `return` wherever just makes the code base
> > less coherent. Especially if it has hidden meanings like 'hey this
> > return means something'.
> >
> > ----------------
> >
> > All in all it's mostly a style convention, which is why Rubocop exists
> > and 1000s of rubyists decide to use it, to stop these discussions from
> > happening and standardize on one style. All major style guides use it
> > (fallacy of appeal to authority, I know), for a reason - so that code
> > looks like it was written by one person and it's more readable.
> >
> > https://github.com/bbatsov/ruby-style-guide
> > https://github.com/airbnb/ruby
> > https://github.com/styleguide/ruby
> >
> > The whole point for that cop to exist is to stop people from discussing
> > about this, and spend that time writing (coherent, homogeneous) code.
> >
> > > I don't see any negatives. Based on PR discussion, I don't understand why
> > > it would require extra effort from reviewer. It's fine when someone uses
> > > it, just ignore it.
> > >
> > > So +1 from me for disabling globally.
> > >
> > > --
> > > Marek
> > >
> > > --
> > > You received this message because you are subscribed to the Google Groups
> > > "foreman-dev" group. To unsubscribe from this group and stop receiving
> > > emails from it, send an email to
> > > foreman-dev+unsubscribe@googlegroups.com. For more options, visit
> > > https://groups.google.com/d/optout.
> >
> > --
> > Daniel Lobato Garcia
> >
> > @dLobatog
> > blog.daniellobato.me
> > daniellobato.me
> >
> > GPG: http://keys.gnupg.net/pks/lookup?op=get&search=0x7A92D6DD38D6DE30
> > Keybase: https://keybase.io/elobato
>
> --
> You received this message because you are subscribed to the Google Groups
> "foreman-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to foreman-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>