Re: Re: [foreman-dev] Hash rocket syntax

[Daniel Lobato Garcia]

On 08/24, Marek Hulán wrote:
> > Also, I am not a huge fan of Github’s style guide since it has some weird
> > things like using TomDoc: https://github.com/styleguide/ruby/documentation
>
> That's fine, we never required this styleguide in our PRs. I'm just saying the
> fact that there is a cop that (even though can be configured either way)
> doesn't mean all ruby devs enable it. Rubocop is not "ruby community style
> guide".

https://github.com/bbatsov/ruby-style-guide which is the closest thing
to a "ruby community style guide" we have, gave birth to Rubocop, it is
meant to be precisely a way to enforce the "ruby community style guide"

--
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: PGP signature

Re: [foreman-dev] Hash rocket syntax

[Tomas Strachota]

On 08/24/2016 05:28 PM, David Davis wrote:

I agree for the most part that Ruby is a TMTOWTDI [1] language but I
think most people would argue for some level of consistency (i.e.
TIMTOWTDIBSCINABTE). A rather extreme example would be indentation. You
can use whatever indentation in Ruby you want but you should probably
try to use the same number of spaces in a project. As such, Rubocop
tries to ensure consistency by allowing you to
configure IndentationWidth cop.

Speaking of consistency: the Cop for HashSyntax can be configured to
prevent mixed syntax. For example, it would prevent:

{ a: 1,
  :b => 2,
  c: 3,
}

Maybe we should at least enforce this?


This is one of cops that make to me.
+1 to forbid mixed syntax but allow both styles otherwise.



[1] https://en.wikipedia.org/wiki/There%27s_more_than_one_way_to_do_it


David

On Wed, Aug 24, 2016 at 10:38 AM, Ewoud Kohl van Wijngaarden
mailto:ew...@kohlvanwijngaarden.nl>> wrote:

On Wed, Aug 24, 2016 at 07:43:31AM -0400, David Davis wrote:
> I would hope that all foreman and foreman-related projects would have some
> level of rubocop checking even if it’s just basic stuff like whitespace (I
> tried to do this for dynflow but closed my PR out after it had conflicts
> and no activity).

Correct me if I'm wrong, but I think the core of the problem is that
some people prefer one and only one way of doing things. You see this in
the Zen of Python[1]:

There should be one-- and preferably only one --obvious way to do it.

Ruby is heavily influenced by Perl which does the opposite and to me
appears to support doing things in as many ways as possible.

This particular case (hash rockets) is allowing logically equal
statements to be written in two different ways. Here the two
philosophies collide causing conflict.

Beyond PEP 8[2] is a related talk that's great. The speaker goes into
simple syntax issues vs real style. While the subject is PEP 8, it
applies to non-Python as well. The summary is embrace the language and
follow the conventions, but focus on the bigger API instead of the
details.

While personally I'm in the Python camp here (pick one way and stick
with it), I don't feel like I submit sufficient code to have a say in
this. My feeling is that there's no logical or quality difference and
Ruby allows both styles which is common among Ruby code. Therefor both
should be allowed.

[1]: https://www.python.org/dev/peps/pep-0020/

[2]: https://www.youtube.com/watch?v=wf-BqAjZb8M


--
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+unsubscr...@googlegroups.com
.
For more options, visit https://groups.google.com/d/optout
.


--
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+unsubscr...@googlegroups.com
.
For more options, visit https://groups.google.com/d/optout.


--
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [foreman-dev] Hash rocket syntax

[Ewoud Kohl van Wijngaarden]

On Wed, Aug 24, 2016 at 11:28:49AM -0400, David Davis wrote:
> I agree for the most part that Ruby is a TMTOWTDI [1] language but I think
> most people would argue for some level of consistency (i.e.
> TIMTOWTDIBSCINABTE). A rather extreme example would be indentation. You can
> use whatever indentation in Ruby you want but you should probably try to
> use the same number of spaces in a project. As such, Rubocop tries to
> ensure consistency by allowing you to configure IndentationWidth cop.

Like I said, I'm mostly a Python guy and prefer a single way so you
don't have to convince me. Pushing that on a Ruby project may be
swimming against the current.

> Speaking of consistency: the Cop for HashSyntax can be configured to
> prevent mixed syntax. For example, it would prevent:
> 
> { a: 1,
>   :b => 2,
>   c: 3,
> }
> 
> Maybe we should at least enforce this?

One style per hash sounds totally reasonable to me.

> [1] https://en.wikipedia.org/wiki/There%27s_more_than_one_way_to_do_it
> 
> 
> David
> 
> On Wed, Aug 24, 2016 at 10:38 AM, Ewoud Kohl van Wijngaarden <
> ew...@kohlvanwijngaarden.nl> wrote:
> 
> > On Wed, Aug 24, 2016 at 07:43:31AM -0400, David Davis wrote:
> > > I would hope that all foreman and foreman-related projects would have
> > some
> > > level of rubocop checking even if it’s just basic stuff like whitespace
> > (I
> > > tried to do this for dynflow but closed my PR out after it had conflicts
> > > and no activity).
> >
> > Correct me if I'm wrong, but I think the core of the problem is that
> > some people prefer one and only one way of doing things. You see this in
> > the Zen of Python[1]:
> >
> > There should be one-- and preferably only one --obvious way to do it.
> >
> > Ruby is heavily influenced by Perl which does the opposite and to me
> > appears to support doing things in as many ways as possible.
> >
> > This particular case (hash rockets) is allowing logically equal
> > statements to be written in two different ways. Here the two
> > philosophies collide causing conflict.
> >
> > Beyond PEP 8[2] is a related talk that's great. The speaker goes into
> > simple syntax issues vs real style. While the subject is PEP 8, it
> > applies to non-Python as well. The summary is embrace the language and
> > follow the conventions, but focus on the bigger API instead of the
> > details.
> >
> > While personally I'm in the Python camp here (pick one way and stick
> > with it), I don't feel like I submit sufficient code to have a say in
> > this. My feeling is that there's no logical or quality difference and
> > Ruby allows both styles which is common among Ruby code. Therefor both
> > should be allowed.
> >
> > [1]: https://www.python.org/dev/peps/pep-0020/
> > [2]: https://www.youtube.com/watch?v=wf-BqAjZb8M
> >
> > --
> > 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+unsubscr...@googlegroups.com.
> > For more options, visit https://groups.google.com/d/optout.
> >
> 
> -- 
> 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+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

-- 
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [foreman-dev] Hash rocket syntax

[David Davis]

I agree for the most part that Ruby is a TMTOWTDI [1] language but I think
most people would argue for some level of consistency (i.e.
TIMTOWTDIBSCINABTE). A rather extreme example would be indentation. You can
use whatever indentation in Ruby you want but you should probably try to
use the same number of spaces in a project. As such, Rubocop tries to
ensure consistency by allowing you to configure IndentationWidth cop.

Speaking of consistency: the Cop for HashSyntax can be configured to
prevent mixed syntax. For example, it would prevent:

{ a: 1,
  :b => 2,
  c: 3,
}

Maybe we should at least enforce this?

[1] https://en.wikipedia.org/wiki/There%27s_more_than_one_way_to_do_it


David

On Wed, Aug 24, 2016 at 10:38 AM, Ewoud Kohl van Wijngaarden <
ew...@kohlvanwijngaarden.nl> wrote:

> On Wed, Aug 24, 2016 at 07:43:31AM -0400, David Davis wrote:
> > I would hope that all foreman and foreman-related projects would have
> some
> > level of rubocop checking even if it’s just basic stuff like whitespace
> (I
> > tried to do this for dynflow but closed my PR out after it had conflicts
> > and no activity).
>
> Correct me if I'm wrong, but I think the core of the problem is that
> some people prefer one and only one way of doing things. You see this in
> the Zen of Python[1]:
>
> There should be one-- and preferably only one --obvious way to do it.
>
> Ruby is heavily influenced by Perl which does the opposite and to me
> appears to support doing things in as many ways as possible.
>
> This particular case (hash rockets) is allowing logically equal
> statements to be written in two different ways. Here the two
> philosophies collide causing conflict.
>
> Beyond PEP 8[2] is a related talk that's great. The speaker goes into
> simple syntax issues vs real style. While the subject is PEP 8, it
> applies to non-Python as well. The summary is embrace the language and
> follow the conventions, but focus on the bigger API instead of the
> details.
>
> While personally I'm in the Python camp here (pick one way and stick
> with it), I don't feel like I submit sufficient code to have a say in
> this. My feeling is that there's no logical or quality difference and
> Ruby allows both styles which is common among Ruby code. Therefor both
> should be allowed.
>
> [1]: https://www.python.org/dev/peps/pep-0020/
> [2]: https://www.youtube.com/watch?v=wf-BqAjZb8M
>
> --
> 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+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [foreman-dev] Hash rocket syntax

[Marek Hulán]

On Wednesday 24 of August 2016 07:43:31 David Davis wrote:
> On Wed, Aug 24, 2016 at 4:49 AM, Marek Hulán  wrote:
> > Few more comments below
> > 
> > > What do you mean by "Foreman dev"?
> > 
> > You, me, everyone who contributed to Foreman.
> 
> So then your statement about not one foreman dev being a rubocop
> contributor is incorrect, right?

Not sure I follow. What I meant is that there's at least one foreman dev (me) 
who does not like at least one cop. If it sounded like there's no single 
foreman dev being rubocop dev at the same time, then I'm sorry, it was not 
what I was trying to say.

> > > I’ve contributed to both foreman and
> > > rubocop several times. Having worked on rubocop with all its cops
> > > enabled
> > > hasn’t been a deterrent to me even though I don’t necessarily agree with
> > > all cops.
> > 
> > Seems rubocop devs are happy with more cops than at least one foreman dev
> > is
> > (me). That make sense, otherwise they would not contribute to rubocop I
> > guess
> > 
> > :-)
> > :
> > > I’m not disagreeing with you. But I still have nightmares about the
> > 
> > dynflow
> > 
> > > source code which had no rubocop checking at all and featured a number
> > > of
> > > obscure ruby syntaxes that rubocop would have forbid.
> > 
> > If dynflow was foreman core project and was reviewed in that way from the
> > first
> > commit, this would not happen. Rubocop would not help too much. I tried to
> > run
> > rubocop on dynflow. From 1346 offenses I found only 22 regarding the
> > obscure
> > syntax, rest is "redundant return", "1.9 hash syntax", "top level class
> > documentation missing" stuff. If you want to reproduce, see [1].
> 
> You’re grepping for lambda and only checking for a handful of lambda cops.
> What about cops like MethodCallParentheses, MultilineIfThen, etc? Even some
> of the cops like SpaceAroundOperators would have prevented stuff like “=->”.

Because these were those that mentioned "=->". Rubopcop found 3 
MethodCallParentheses and 1 MultilineIfThen, looking at the code it points to, 
I don't consider them obscure syntax.

> I would hope that all foreman and foreman-related projects would have some
> level of rubocop checking even if it’s just basic stuff like whitespace (I
> tried to do this for dynflow but closed my PR out after it had conflicts
> and no activity).
> 
> > This thread started around has rocket syntax. So to remain on topic, I
> > don't
> > think that the fact there is a cop for enforcing one hash syntax, it's
> > widely
> > accepted among Ruby devs. Speaking of hash rockets, an example might be
> > github
> > style guide - https://github.com/styleguide/ruby/hashes
> 
> That’s not true. The HashSyntax cop can be used to check for hash rockets:
> https://git.io/v67yi

What's not true?

> My vote is not to enforce hash syntax—let developers choose whether to use
> 1.9 vs hash rockets.

I vote for the same, in other words we don't need this being enforced by cop 
even though it exists. And as you said previously, we don't need to enforce it 
in PR reviews.

> Also, I am not a huge fan of Github’s style guide since it has some weird
> things like using TomDoc: https://github.com/styleguide/ruby/documentation

That's fine, we never required this styleguide in our PRs. I'm just saying the 
fact that there is a cop that (even though can be configured either way) 
doesn't mean all ruby devs enable it. Rubocop is not "ruby community style 
guide".

--
Marek

> 
> > [1] https://gist.github.com/ares/ff6e8b19361148e2be17290f1167c7c4
> > 
> > --
> > Marek
> > 
> > > I think there’s probably a happy medium between not using rubocop and
> > > enabling all rubocop cops.
> > > 
> > > 
> > > David
> > > 
> > > On Tue, Aug 23, 2016 at 11:22 AM, Marek Hulán  wrote:
> > > > On Tuesday 23 of August 2016 07:47:31 David Davis wrote:
> > > > > The plugins may not have as many contributors as foreman core but
> > > > > rubocop
> > > > > has more contributors (279 vs 200 for foreman) and they have pretty
> > 
> > much
> > 
> > > > > all cops enabled. It’s only been around for 3 years (vs 7 years for
> > > > > foreman) so it doesn’t seem to be a deterrent to community
> > > > > contributions.
> > > > 
> > > > I don't think this is fair comparison. Do you think that 279
> > 
> > contributors
> > 
> > > > of
> > > > one project represent all Ruby devs or all Foreman devs? At least not
> > 
> > one
> > 
> > > > Foreman dev, that's for sure. I don't say we should disable rubocop
> > 
> > but I
> > 
> > > > don't think all devs are happy with all cops. Adding cops like hash
> > 
> > rocket
> > 
> > > > makes it only worse.
> > > > 
> > > > --
> > > > Marek
> > > > 
> > > > > I’d probably wager that most experienced Ruby developers are aware
> > > > > of
> > > > > rubocop and the ruby community style guide [1]. For unexperienced
> > 
> > Ruby
> > 
> > > > > developers, I think they are a great way to learn good Ruby coding
> > > > > standards.
> > > > > 
> > > > > That said, I agree i

Re: [foreman-dev] Hash rocket syntax

[Ewoud Kohl van Wijngaarden]

On Wed, Aug 24, 2016 at 07:43:31AM -0400, David Davis wrote:
> I would hope that all foreman and foreman-related projects would have some
> level of rubocop checking even if it’s just basic stuff like whitespace (I
> tried to do this for dynflow but closed my PR out after it had conflicts
> and no activity).

Correct me if I'm wrong, but I think the core of the problem is that
some people prefer one and only one way of doing things. You see this in
the Zen of Python[1]:

There should be one-- and preferably only one --obvious way to do it.

Ruby is heavily influenced by Perl which does the opposite and to me
appears to support doing things in as many ways as possible.

This particular case (hash rockets) is allowing logically equal
statements to be written in two different ways. Here the two
philosophies collide causing conflict.

Beyond PEP 8[2] is a related talk that's great. The speaker goes into
simple syntax issues vs real style. While the subject is PEP 8, it
applies to non-Python as well. The summary is embrace the language and
follow the conventions, but focus on the bigger API instead of the
details.

While personally I'm in the Python camp here (pick one way and stick
with it), I don't feel like I submit sufficient code to have a say in
this. My feeling is that there's no logical or quality difference and
Ruby allows both styles which is common among Ruby code. Therefor both
should be allowed.

[1]: https://www.python.org/dev/peps/pep-0020/
[2]: https://www.youtube.com/watch?v=wf-BqAjZb8M

-- 
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [foreman-dev] Hash rocket syntax

[David Davis]

I was thinking about responding to the items in your email but I feel like
I’ve put enough time into this discussion already. Just some general
thoughts.

I think you’re mischaracterizing my argument. I am not arguing that we
enable all cops. In fact, in the dynflow case I tried to open a PR with
just white spacing checking enabled but closed it out after it got out of
sync and had no activity:

https://github.com/Dynflow/dynflow/pull/169

To your point about disabling rubocop per instance, I’m sure that reduces
readability. I guess that’s subjective.

And we did have discussions on foreman-dev about which cops to
enable/disable. We even had a poll. The resulting PR reached an impasse
though even though we had reached a consensus on foreman-dev.


David

On Wed, Aug 24, 2016 at 7:41 AM, Ivan Necas  wrote:

> On Wed, Aug 24, 2016 at 10:49 AM, Marek Hulán  wrote:
> > Few more comments below
> >
> >> What do you mean by "Foreman dev"?
> >
> > You, me, everyone who contributed to Foreman.
> >
> >> I’ve contributed to both foreman and
> >> rubocop several times. Having worked on rubocop with all its cops
> enabled
> >> hasn’t been a deterrent to me even though I don’t necessarily agree with
> >> all cops.
> >
> > Seems rubocop devs are happy with more cops than at least one foreman
> dev is
> > (me). That make sense, otherwise they would not contribute to rubocop I
> guess
> > :-)
> >
> >> I’m not disagreeing with you. But I still have nightmares about the
> dynflow
> >> source code which had no rubocop checking at all and featured a number
> of
> >> obscure ruby syntaxes that rubocop would have forbid.
>
> I'm sorry, but this seem like a straw man argument to me. I agree with
> what Marek said.
> The think with the obscure syntax a little to do with Rubocop,
> but rather using the Algebrick gem. We gave it a try, it didn't work,
> we will remove eventually.
> Nothing to do with Rubocop. I'm not saying Dynflow would not benefit
> from Rubocop.
> And once more prio things get sorted out, we will finish that. I would
> not enable all cops
> though.
>
> I think we got quite far from the Hash rocket topic, just my last
> comment on Rubocop
> in general. What I see here is people saying "the more Rubocop" = "the
> more consistency"
> = "the more readable code". I don't agree with this reasoning. Yes,
> some cops increases
> the consistency, but it can just lead to your code to be consistently
> unreadable. And some cops
> have nothing to do with consistency.
>
> Let's take few examples from the Rubocop source code itself:
>
>   https://github.com/bbatsov/rubocop/blob/v0.42.0/lib/
> rubocop/config.rb#L243
> - multi-line modifier statement - I hope nobody here would argue
> for this being more readable
> then standard if
>
>   https://github.com/bbatsov/rubocop/blob/v0.42.0/lib/
> rubocop/config_loader.rb#L74
> - combination of modifier and non-modifier if: if you tried to put
> everything into non-modifier form,
>   it would tell you that you should use the modifier one (I
> tried:), which would eventually lead you
>   to the same case as previous one
>
>   https://github.com/bbatsov/rubocop/blob/v0.42.0/lib/rubocop/config.rb
>  - IMO the 80 lines limit just makes too many lines spread across
> more lines, making it harder to read;
>ok, I know some people us vim in split on lower resolution, but
> I don't think others and the readability
>should suffer from that: I would take the max line size from
> what actually helps the readability, not
>what resolution somebody has, otherwise, I would just argue
> that we need max 50 lines, to make it
>easier for me to review the code on my phone.
>
>https://github.com/bbatsov/rubocop/blob/v0.42.0/lib/rubocop/cli.rb#L5
>   - the comment says "The CLI is a class responsible of handling
> all the command line interface logic"
>   - while it was true when the class was added few years ago, it's
> no more valid that it's responsible
> for handling *all* of the logic, because the option parser is
> in separate class now.
>   - I would say the comment was just useless at the first place,
> and got obsolete (as comments
> use to) and the only reason people add it there is the policy.
>   - also, from personal experience with Rubocop documentation, I
> would not say it helped anyhow,
> I feel the pain every time I try to find what some cop expects
> or how to disable that, I end up
> looking at rubocop tests anyway usually.
>
> What I think is happening is people relying on Rubocop keeping their
> code readable
> and not paying any attention to it. And some cops make it also harder
> to improve the readability,
> (I know you can always disable the the cop per line, but I would say
> that already affects
> the readability in a bad way).
>
> I know that it might be frustrating for someone to discuss the styling
> between
> the developers, rather that aligning to whatever few peo

Re: [foreman-dev] Hash rocket syntax

[David Davis]

On Wed, Aug 24, 2016 at 4:49 AM, Marek Hulán  wrote:

> Few more comments below
>
> > What do you mean by "Foreman dev"?
>
> You, me, everyone who contributed to Foreman.
>

So then your statement about not one foreman dev being a rubocop
contributor is incorrect, right?



>
> > I’ve contributed to both foreman and
> > rubocop several times. Having worked on rubocop with all its cops enabled
> > hasn’t been a deterrent to me even though I don’t necessarily agree with
> > all cops.
>
> Seems rubocop devs are happy with more cops than at least one foreman dev
> is
> (me). That make sense, otherwise they would not contribute to rubocop I
> guess
> :-)
>
> > I’m not disagreeing with you. But I still have nightmares about the
> dynflow
> > source code which had no rubocop checking at all and featured a number of
> > obscure ruby syntaxes that rubocop would have forbid.
>
> If dynflow was foreman core project and was reviewed in that way from the
> first
> commit, this would not happen. Rubocop would not help too much. I tried to
> run
> rubocop on dynflow. From 1346 offenses I found only 22 regarding the
> obscure
> syntax, rest is "redundant return", "1.9 hash syntax", "top level class
> documentation missing" stuff. If you want to reproduce, see [1].
>
>
You’re grepping for lambda and only checking for a handful of lambda cops.
What about cops like MethodCallParentheses, MultilineIfThen, etc? Even some
of the cops like SpaceAroundOperators would have prevented stuff like “=->”.

I would hope that all foreman and foreman-related projects would have some
level of rubocop checking even if it’s just basic stuff like whitespace (I
tried to do this for dynflow but closed my PR out after it had conflicts
and no activity).



> This thread started around has rocket syntax. So to remain on topic, I
> don't
> think that the fact there is a cop for enforcing one hash syntax, it's
> widely
> accepted among Ruby devs. Speaking of hash rockets, an example might be
> github
> style guide - https://github.com/styleguide/ruby/hashes


That’s not true. The HashSyntax cop can be used to check for hash rockets:
https://git.io/v67yi

My vote is not to enforce hash syntax—let developers choose whether to use
1.9 vs hash rockets.

Also, I am not a huge fan of Github’s style guide since it has some weird
things like using TomDoc: https://github.com/styleguide/ruby/documentation


>
>
> [1] https://gist.github.com/ares/ff6e8b19361148e2be17290f1167c7c4
>
> --
> Marek
>
> > I think there’s probably a happy medium between not using rubocop and
> > enabling all rubocop cops.
> >
> >
> > David
> >
> > On Tue, Aug 23, 2016 at 11:22 AM, Marek Hulán  wrote:
> > > On Tuesday 23 of August 2016 07:47:31 David Davis wrote:
> > > > The plugins may not have as many contributors as foreman core but
> > > > rubocop
> > > > has more contributors (279 vs 200 for foreman) and they have pretty
> much
> > > > all cops enabled. It’s only been around for 3 years (vs 7 years for
> > > > foreman) so it doesn’t seem to be a deterrent to community
> > > > contributions.
> > >
> > > I don't think this is fair comparison. Do you think that 279
> contributors
> > > of
> > > one project represent all Ruby devs or all Foreman devs? At least not
> one
> > > Foreman dev, that's for sure. I don't say we should disable rubocop
> but I
> > > don't think all devs are happy with all cops. Adding cops like hash
> rocket
> > > makes it only worse.
> > >
> > > --
> > > Marek
> > >
> > > > I’d probably wager that most experienced Ruby developers are aware of
> > > > rubocop and the ruby community style guide [1]. For unexperienced
> Ruby
> > > > developers, I think they are a great way to learn good Ruby coding
> > > > standards.
> > > >
> > > > That said, I agree it would be nice to get opinions of community
> members
> > > > who contribute to foreman and their experiences with rubocop.
> > > >
> > > > [1] https://github.com/bbatsov/ruby-style-guide
> > > >
> > > >
> > > > David
> > > >
> > > > On Tue, Aug 23, 2016 at 3:48 AM, Marek Hulán 
> wrote:
> > > > > I'm with Lukas, we already enforce too many things without clear
> > >
> > > benefit.
> > >
> > > > > Let's
> > > > > keep both ways accepted which is default for all rubyists.
> > > > >
> > > > > > > I think we implemented Rubocop far beyond what's reasonable
> point.
> > >
> > > It
> > >
> > > > > > > make sense for dangerous constructs, but not in this case (and
> few
> > > > > > > others).
> > > > >
> > > > > +1, since rubocop leads to bike-shedding and annoyed devs from both
> > >
> > > sides
> > >
> > > > > I
> > > > > don't think it's a good idea to introduce more cops.
> > > > >
> > > > > > I'd argue the opposite, in foreman_docker or foreman_ansible I
> think
> > > > > > rubocop (with *all* cops enabled) helped maintain good coding
> > >
> > > standards
> > >
> > > > > > immensely and making the project *much easier* to read and get
> used
> > >
> > > to.
> > >
> > > > > With all respect, these plugins do not h

Re: [foreman-dev] Hash rocket syntax

[Ivan Necas]

On Wed, Aug 24, 2016 at 10:49 AM, Marek Hulán  wrote:
> Few more comments below
>
>> What do you mean by "Foreman dev"?
>
> You, me, everyone who contributed to Foreman.
>
>> I’ve contributed to both foreman and
>> rubocop several times. Having worked on rubocop with all its cops enabled
>> hasn’t been a deterrent to me even though I don’t necessarily agree with
>> all cops.
>
> Seems rubocop devs are happy with more cops than at least one foreman dev is
> (me). That make sense, otherwise they would not contribute to rubocop I guess
> :-)
>
>> I’m not disagreeing with you. But I still have nightmares about the dynflow
>> source code which had no rubocop checking at all and featured a number of
>> obscure ruby syntaxes that rubocop would have forbid.

I'm sorry, but this seem like a straw man argument to me. I agree with
what Marek said.
The think with the obscure syntax a little to do with Rubocop,
but rather using the Algebrick gem. We gave it a try, it didn't work,
we will remove eventually.
Nothing to do with Rubocop. I'm not saying Dynflow would not benefit
from Rubocop.
And once more prio things get sorted out, we will finish that. I would
not enable all cops
though.

I think we got quite far from the Hash rocket topic, just my last
comment on Rubocop
in general. What I see here is people saying "the more Rubocop" = "the
more consistency"
= "the more readable code". I don't agree with this reasoning. Yes,
some cops increases
the consistency, but it can just lead to your code to be consistently
unreadable. And some cops
have nothing to do with consistency.

Let's take few examples from the Rubocop source code itself:

  https://github.com/bbatsov/rubocop/blob/v0.42.0/lib/rubocop/config.rb#L243
- multi-line modifier statement - I hope nobody here would argue
for this being more readable
then standard if

  
https://github.com/bbatsov/rubocop/blob/v0.42.0/lib/rubocop/config_loader.rb#L74
- combination of modifier and non-modifier if: if you tried to put
everything into non-modifier form,
  it would tell you that you should use the modifier one (I
tried:), which would eventually lead you
  to the same case as previous one

  https://github.com/bbatsov/rubocop/blob/v0.42.0/lib/rubocop/config.rb
 - IMO the 80 lines limit just makes too many lines spread across
more lines, making it harder to read;
   ok, I know some people us vim in split on lower resolution, but
I don't think others and the readability
   should suffer from that: I would take the max line size from
what actually helps the readability, not
   what resolution somebody has, otherwise, I would just argue
that we need max 50 lines, to make it
   easier for me to review the code on my phone.

   https://github.com/bbatsov/rubocop/blob/v0.42.0/lib/rubocop/cli.rb#L5
  - the comment says "The CLI is a class responsible of handling
all the command line interface logic"
  - while it was true when the class was added few years ago, it's
no more valid that it's responsible
for handling *all* of the logic, because the option parser is
in separate class now.
  - I would say the comment was just useless at the first place,
and got obsolete (as comments
use to) and the only reason people add it there is the policy.
  - also, from personal experience with Rubocop documentation, I
would not say it helped anyhow,
I feel the pain every time I try to find what some cop expects
or how to disable that, I end up
looking at rubocop tests anyway usually.

What I think is happening is people relying on Rubocop keeping their
code readable
and not paying any attention to it. And some cops make it also harder
to improve the readability,
(I know you can always disable the the cop per line, but I would say
that already affects
the readability in a bad way).

I know that it might be frustrating for someone to discuss the styling between
the developers, rather that aligning to whatever few people agree on Rubocop
PR to merge. I asked several times to have proper discussion on the
cops to enable/disable,
rather than doing that in PRs directly. It didn't happened, which is
why we are arguing about
it every single time somebody wants to introduce some new one.

-- Ivan

>
> If dynflow was foreman core project and was reviewed in that way from the 
> first
> commit, this would not happen. Rubocop would not help too much. I tried to run
> rubocop on dynflow. From 1346 offenses I found only 22 regarding the obscure
> syntax, rest is "redundant return", "1.9 hash syntax", "top level class
> documentation missing" stuff. If you want to reproduce, see [1].
>
> This thread started around has rocket syntax. So to remain on topic, I don't
> think that the fact there is a cop for enforcing one hash syntax, it's widely
> accepted among Ruby devs. Speaking of hash rockets, an example might be github
> style guide - https://github.com/styleguide/ruby/hashes
>
> [1] https://gist.github.com/ares/ff6e8

Re: [foreman-dev] Hash rocket syntax

[Marek Hulán]

Few more comments below

> What do you mean by "Foreman dev"? 

You, me, everyone who contributed to Foreman.

> I’ve contributed to both foreman and
> rubocop several times. Having worked on rubocop with all its cops enabled
> hasn’t been a deterrent to me even though I don’t necessarily agree with
> all cops.

Seems rubocop devs are happy with more cops than at least one foreman dev is 
(me). That make sense, otherwise they would not contribute to rubocop I guess 
:-)

> I’m not disagreeing with you. But I still have nightmares about the dynflow
> source code which had no rubocop checking at all and featured a number of
> obscure ruby syntaxes that rubocop would have forbid.

If dynflow was foreman core project and was reviewed in that way from the first 
commit, this would not happen. Rubocop would not help too much. I tried to run 
rubocop on dynflow. From 1346 offenses I found only 22 regarding the obscure 
syntax, rest is "redundant return", "1.9 hash syntax", "top level class 
documentation missing" stuff. If you want to reproduce, see [1].

This thread started around has rocket syntax. So to remain on topic, I don't 
think that the fact there is a cop for enforcing one hash syntax, it's widely 
accepted among Ruby devs. Speaking of hash rockets, an example might be github 
style guide - https://github.com/styleguide/ruby/hashes

[1] https://gist.github.com/ares/ff6e8b19361148e2be17290f1167c7c4

--
Marek

> I think there’s probably a happy medium between not using rubocop and
> enabling all rubocop cops.
> 
> 
> David
> 
> On Tue, Aug 23, 2016 at 11:22 AM, Marek Hulán  wrote:
> > On Tuesday 23 of August 2016 07:47:31 David Davis wrote:
> > > The plugins may not have as many contributors as foreman core but
> > > rubocop
> > > has more contributors (279 vs 200 for foreman) and they have pretty much
> > > all cops enabled. It’s only been around for 3 years (vs 7 years for
> > > foreman) so it doesn’t seem to be a deterrent to community
> > > contributions.
> > 
> > I don't think this is fair comparison. Do you think that 279 contributors
> > of
> > one project represent all Ruby devs or all Foreman devs? At least not one
> > Foreman dev, that's for sure. I don't say we should disable rubocop but I
> > don't think all devs are happy with all cops. Adding cops like hash rocket
> > makes it only worse.
> > 
> > --
> > Marek
> > 
> > > I’d probably wager that most experienced Ruby developers are aware of
> > > rubocop and the ruby community style guide [1]. For unexperienced Ruby
> > > developers, I think they are a great way to learn good Ruby coding
> > > standards.
> > > 
> > > That said, I agree it would be nice to get opinions of community members
> > > who contribute to foreman and their experiences with rubocop.
> > > 
> > > [1] https://github.com/bbatsov/ruby-style-guide
> > > 
> > > 
> > > David
> > > 
> > > On Tue, Aug 23, 2016 at 3:48 AM, Marek Hulán  wrote:
> > > > I'm with Lukas, we already enforce too many things without clear
> > 
> > benefit.
> > 
> > > > Let's
> > > > keep both ways accepted which is default for all rubyists.
> > > > 
> > > > > > I think we implemented Rubocop far beyond what's reasonable point.
> > 
> > It
> > 
> > > > > > make sense for dangerous constructs, but not in this case (and few
> > > > > > others).
> > > > 
> > > > +1, since rubocop leads to bike-shedding and annoyed devs from both
> > 
> > sides
> > 
> > > > I
> > > > don't think it's a good idea to introduce more cops.
> > > > 
> > > > > I'd argue the opposite, in foreman_docker or foreman_ansible I think
> > > > > rubocop (with *all* cops enabled) helped maintain good coding
> > 
> > standards
> > 
> > > > > immensely and making the project *much easier* to read and get used
> > 
> > to.
> > 
> > > > With all respect, these plugins do not have as many contributors as
> > > > Foreman
> > > > and I'd like to hear from these contributors whether rubocop helped or
> > > > annoyed
> > > > them. I don't think that all cops enabled == good coding standards.
> > 
> > And it
> > 
> > > > does not imply good readability, that's on reviewer. TBH I think it's
> > 
> > also
> > 
> > > > pretty subjective (remember explicit return cop discussion).
> > > > 
> > > > --
> > > > Marek
> > > > 
> > > > On Friday 19 of August 2016 11:54:23 Daniel Lobato Garcia wrote:
> > > > > On 08/19, Lukas Zapletal wrote:
> > > > > > > As discussed on IRC yesterday there should be consistency and
> > 
> > there
> > 
> > > > is
> > > > 
> > > > > > > an
> > > > > > > option to autofix with rubocop if the style is changed to change
> > > > > > > existing
> > > > > > > code with less effort.
> > > > > > 
> > > > > > TL;DR - Let's keep Rubocop away from rockethash thing.
> > > > > > 
> > > > > > What the consistency gives us? We all know there are two ways and
> > 
> > both
> > 
> > > > > > will work. Let's avoid big bangs that will make cherry picking
> > 
> > harder
> > 
> > > > > > and just let's slowly improve as the time goes on.
> > > > >

Re: [foreman-dev] Hash rocket syntax

[Greg Sutcliffe]

On 23 August 2016 at 16:32, David Davis  wrote:

> I think there’s probably a happy medium between not using rubocop and
> enabling all rubocop cops.
>

This. Speaking as an ex-sysadmin who tends to write functional but usually
horrible code, Rubocop has been helpful to me in ensuring the consistency
of what I write, and the complexity cops have encouraged me to
rethink/refactor parts of my plugins. It definitely has a place.

At one end, the big cops like complexity, length etc help me to produce
good code. At the other end, the obvious low-fruit wins like whitespace
consistency are fine. But in the middle is a grey area, and here I agree
with Marek - it feels like nitpicking for the sake of nitpicking. Hash
rockets feels like of of these nitpicks - it doesn't affect the quality of
the code, and it's not a trivial change that's easy to agree on (as
demonstrated).

It's worth noting you can always add your own Rubocop yml file, it has a
search path - so if you want to be more strict with yourself, you can :)

$0.02 deposited :P

Greg

-- 
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [foreman-dev] Hash rocket syntax

[David Davis]

What do you mean by "Foreman dev"? I’ve contributed to both foreman and
rubocop several times. Having worked on rubocop with all its cops enabled
hasn’t been a deterrent to me even though I don’t necessarily agree with
all cops.

I’m not disagreeing with you. But I still have nightmares about the dynflow
source code which had no rubocop checking at all and featured a number of
obscure ruby syntaxes that rubocop would have forbid.

I think there’s probably a happy medium between not using rubocop and
enabling all rubocop cops.


David

On Tue, Aug 23, 2016 at 11:22 AM, Marek Hulán  wrote:

> On Tuesday 23 of August 2016 07:47:31 David Davis wrote:
> > The plugins may not have as many contributors as foreman core but rubocop
> > has more contributors (279 vs 200 for foreman) and they have pretty much
> > all cops enabled. It’s only been around for 3 years (vs 7 years for
> > foreman) so it doesn’t seem to be a deterrent to community contributions.
>
> I don't think this is fair comparison. Do you think that 279 contributors
> of
> one project represent all Ruby devs or all Foreman devs? At least not one
> Foreman dev, that's for sure. I don't say we should disable rubocop but I
> don't think all devs are happy with all cops. Adding cops like hash rocket
> makes it only worse.
>
> --
> Marek
>
> >
> > I’d probably wager that most experienced Ruby developers are aware of
> > rubocop and the ruby community style guide [1]. For unexperienced Ruby
> > developers, I think they are a great way to learn good Ruby coding
> > standards.
> >
> > That said, I agree it would be nice to get opinions of community members
> > who contribute to foreman and their experiences with rubocop.
> >
> > [1] https://github.com/bbatsov/ruby-style-guide
> >
> >
> > David
> >
> > On Tue, Aug 23, 2016 at 3:48 AM, Marek Hulán  wrote:
> > > I'm with Lukas, we already enforce too many things without clear
> benefit.
> > > Let's
> > > keep both ways accepted which is default for all rubyists.
> > >
> > > > > I think we implemented Rubocop far beyond what's reasonable point.
> It
> > > > > make sense for dangerous constructs, but not in this case (and few
> > > > > others).
> > >
> > > +1, since rubocop leads to bike-shedding and annoyed devs from both
> sides
> > > I
> > > don't think it's a good idea to introduce more cops.
> > >
> > > > I'd argue the opposite, in foreman_docker or foreman_ansible I think
> > > > rubocop (with *all* cops enabled) helped maintain good coding
> standards
> > > > immensely and making the project *much easier* to read and get used
> to.
> > >
> > > With all respect, these plugins do not have as many contributors as
> > > Foreman
> > > and I'd like to hear from these contributors whether rubocop helped or
> > > annoyed
> > > them. I don't think that all cops enabled == good coding standards.
> And it
> > > does not imply good readability, that's on reviewer. TBH I think it's
> also
> > > pretty subjective (remember explicit return cop discussion).
> > >
> > > --
> > > Marek
> > >
> > > On Friday 19 of August 2016 11:54:23 Daniel Lobato Garcia wrote:
> > > > On 08/19, Lukas Zapletal wrote:
> > > > > > As discussed on IRC yesterday there should be consistency and
> there
> > >
> > > is
> > >
> > > > > > an
> > > > > > option to autofix with rubocop if the style is changed to change
> > > > > > existing
> > > > > > code with less effort.
> > > > >
> > > > > TL;DR - Let's keep Rubocop away from rockethash thing.
> > > > >
> > > > > What the consistency gives us? We all know there are two ways and
> both
> > > > > will work. Let's avoid big bangs that will make cherry picking
> harder
> > > > > and just let's slowly improve as the time goes on.
> > > >
> > > > Not sure what cherry-picking becomes harder after this change? It's
> just
> > > > that people might have to rebase their PRs? That's a small price to
> pay
> > > > considering code is read 1000x more often than written.
> > > >
> > > > > I see no point in changing a single line of code from old to new
> > > > > syntax
> > > > > just for that. We should only change it when changing logic.
> > > > >
> > > > > Even if Rubocop is able to check only for changed lines, I won't
> like
> > > > > that at all. I do not want to switch my brain between Smart Proxy
> and
> > > > > Foreman Core codebases. Both ways should work and be accepted.
> Let's
> > > > > only make the old syntax preferable when reviewing and that's it.
> > > >
> > > > Precisely that's the painpoint, reviewers shouldn't have to pay
> > > > attention to that. One of the points for having style guidelines is
> that
> > > > the code looks and reads as if it had been written by one person.
> > > >
> > > > Think of it from the POV of the ocassional contributor who is just
> > > > confused about which syntax to use because the code mixes them both
> for
> > > > no reason.
> > > >
> > > > > I think we implemented Rubocop far beyond what's reasonable point.
> It
> > > > > make sense for dangerous constructs, 

Re: [foreman-dev] Hash rocket syntax

[Marek Hulán]

On Tuesday 23 of August 2016 07:47:31 David Davis wrote:
> The plugins may not have as many contributors as foreman core but rubocop
> has more contributors (279 vs 200 for foreman) and they have pretty much
> all cops enabled. It’s only been around for 3 years (vs 7 years for
> foreman) so it doesn’t seem to be a deterrent to community contributions.

I don't think this is fair comparison. Do you think that 279 contributors of 
one project represent all Ruby devs or all Foreman devs? At least not one 
Foreman dev, that's for sure. I don't say we should disable rubocop but I 
don't think all devs are happy with all cops. Adding cops like hash rocket 
makes it only worse.

--
Marek

> 
> I’d probably wager that most experienced Ruby developers are aware of
> rubocop and the ruby community style guide [1]. For unexperienced Ruby
> developers, I think they are a great way to learn good Ruby coding
> standards.
> 
> That said, I agree it would be nice to get opinions of community members
> who contribute to foreman and their experiences with rubocop.
> 
> [1] https://github.com/bbatsov/ruby-style-guide
> 
> 
> David
> 
> On Tue, Aug 23, 2016 at 3:48 AM, Marek Hulán  wrote:
> > I'm with Lukas, we already enforce too many things without clear benefit.
> > Let's
> > keep both ways accepted which is default for all rubyists.
> > 
> > > > I think we implemented Rubocop far beyond what's reasonable point. It
> > > > make sense for dangerous constructs, but not in this case (and few
> > > > others).
> > 
> > +1, since rubocop leads to bike-shedding and annoyed devs from both sides
> > I
> > don't think it's a good idea to introduce more cops.
> > 
> > > I'd argue the opposite, in foreman_docker or foreman_ansible I think
> > > rubocop (with *all* cops enabled) helped maintain good coding standards
> > > immensely and making the project *much easier* to read and get used to.
> > 
> > With all respect, these plugins do not have as many contributors as
> > Foreman
> > and I'd like to hear from these contributors whether rubocop helped or
> > annoyed
> > them. I don't think that all cops enabled == good coding standards. And it
> > does not imply good readability, that's on reviewer. TBH I think it's also
> > pretty subjective (remember explicit return cop discussion).
> > 
> > --
> > Marek
> > 
> > On Friday 19 of August 2016 11:54:23 Daniel Lobato Garcia wrote:
> > > On 08/19, Lukas Zapletal wrote:
> > > > > As discussed on IRC yesterday there should be consistency and there
> > 
> > is
> > 
> > > > > an
> > > > > option to autofix with rubocop if the style is changed to change
> > > > > existing
> > > > > code with less effort.
> > > > 
> > > > TL;DR - Let's keep Rubocop away from rockethash thing.
> > > > 
> > > > What the consistency gives us? We all know there are two ways and both
> > > > will work. Let's avoid big bangs that will make cherry picking harder
> > > > and just let's slowly improve as the time goes on.
> > > 
> > > Not sure what cherry-picking becomes harder after this change? It's just
> > > that people might have to rebase their PRs? That's a small price to pay
> > > considering code is read 1000x more often than written.
> > > 
> > > > I see no point in changing a single line of code from old to new
> > > > syntax
> > > > just for that. We should only change it when changing logic.
> > > > 
> > > > Even if Rubocop is able to check only for changed lines, I won't like
> > > > that at all. I do not want to switch my brain between Smart Proxy and
> > > > Foreman Core codebases. Both ways should work and be accepted. Let's
> > > > only make the old syntax preferable when reviewing and that's it.
> > > 
> > > Precisely that's the painpoint, reviewers shouldn't have to pay
> > > attention to that. One of the points for having style guidelines is that
> > > the code looks and reads as if it had been written by one person.
> > > 
> > > Think of it from the POV of the ocassional contributor who is just
> > > confused about which syntax to use because the code mixes them both for
> > > no reason.
> > > 
> > > > I think we implemented Rubocop far beyond what's reasonable point. It
> > > > make sense for dangerous constructs, but not in this case (and few
> > > > others).
> > > 
> > > I'd argue the opposite, in foreman_docker or foreman_ansible I think
> > > rubocop (with *all* cops enabled) helped maintain good coding standards
> > > immensely and making the project *much easier* to read and get used to.
> > > 
> > > Again I think we're really bikeshedding when the purpose of a style
> > > guide is to stop it and make code look more homogeneous
> > > 
> > > > --
> > > > Later,
> > > > 
> > > >  Lukas #lzap Zapletal
> > > > 
> > > > --
> > > > 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+unsubscr...@googlegroups.com. For more opti

Re: [foreman-dev] Hash rocket syntax

[David Davis]

The plugins may not have as many contributors as foreman core but rubocop
has more contributors (279 vs 200 for foreman) and they have pretty much
all cops enabled. It’s only been around for 3 years (vs 7 years for
foreman) so it doesn’t seem to be a deterrent to community contributions.

I’d probably wager that most experienced Ruby developers are aware of
rubocop and the ruby community style guide [1]. For unexperienced Ruby
developers, I think they are a great way to learn good Ruby coding
standards.

That said, I agree it would be nice to get opinions of community members
who contribute to foreman and their experiences with rubocop.

[1] https://github.com/bbatsov/ruby-style-guide


David

On Tue, Aug 23, 2016 at 3:48 AM, Marek Hulán  wrote:

> I'm with Lukas, we already enforce too many things without clear benefit.
> Let's
> keep both ways accepted which is default for all rubyists.
>
> > > I think we implemented Rubocop far beyond what's reasonable point. It
> > > make sense for dangerous constructs, but not in this case (and few
> > > others).
>
> +1, since rubocop leads to bike-shedding and annoyed devs from both sides I
> don't think it's a good idea to introduce more cops.
>
> > I'd argue the opposite, in foreman_docker or foreman_ansible I think
> > rubocop (with *all* cops enabled) helped maintain good coding standards
> > immensely and making the project *much easier* to read and get used to.
>
> With all respect, these plugins do not have as many contributors as Foreman
> and I'd like to hear from these contributors whether rubocop helped or
> annoyed
> them. I don't think that all cops enabled == good coding standards. And it
> does not imply good readability, that's on reviewer. TBH I think it's also
> pretty subjective (remember explicit return cop discussion).
>
> --
> Marek
>
> On Friday 19 of August 2016 11:54:23 Daniel Lobato Garcia wrote:
> > On 08/19, Lukas Zapletal wrote:
> > > > As discussed on IRC yesterday there should be consistency and there
> is
> > > > an
> > > > option to autofix with rubocop if the style is changed to change
> > > > existing
> > > > code with less effort.
> > >
> > > TL;DR - Let's keep Rubocop away from rockethash thing.
> > >
> > > What the consistency gives us? We all know there are two ways and both
> > > will work. Let's avoid big bangs that will make cherry picking harder
> > > and just let's slowly improve as the time goes on.
> >
> > Not sure what cherry-picking becomes harder after this change? It's just
> > that people might have to rebase their PRs? That's a small price to pay
> > considering code is read 1000x more often than written.
> >
> > > I see no point in changing a single line of code from old to new syntax
> > > just for that. We should only change it when changing logic.
> > >
> > > Even if Rubocop is able to check only for changed lines, I won't like
> > > that at all. I do not want to switch my brain between Smart Proxy and
> > > Foreman Core codebases. Both ways should work and be accepted. Let's
> > > only make the old syntax preferable when reviewing and that's it.
> >
> > Precisely that's the painpoint, reviewers shouldn't have to pay
> > attention to that. One of the points for having style guidelines is that
> > the code looks and reads as if it had been written by one person.
> >
> > Think of it from the POV of the ocassional contributor who is just
> > confused about which syntax to use because the code mixes them both for
> > no reason.
> >
> > > I think we implemented Rubocop far beyond what's reasonable point. It
> > > make sense for dangerous constructs, but not in this case (and few
> > > others).
> >
> > I'd argue the opposite, in foreman_docker or foreman_ansible I think
> > rubocop (with *all* cops enabled) helped maintain good coding standards
> > immensely and making the project *much easier* to read and get used to.
> >
> > Again I think we're really bikeshedding when the purpose of a style
> > guide is to stop it and make code look more homogeneous
> >
> > > --
> > > Later,
> > >
> > >  Lukas #lzap Zapletal
> > >
> > > --
> > > 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+unsubscr...@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+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups 

Re: [foreman-dev] Hash rocket syntax

[Marek Hulán]

I'm with Lukas, we already enforce too many things without clear benefit. Let's 
keep both ways accepted which is default for all rubyists.

> > I think we implemented Rubocop far beyond what's reasonable point. It
> > make sense for dangerous constructs, but not in this case (and few
> > others).

+1, since rubocop leads to bike-shedding and annoyed devs from both sides I 
don't think it's a good idea to introduce more cops.

> I'd argue the opposite, in foreman_docker or foreman_ansible I think
> rubocop (with *all* cops enabled) helped maintain good coding standards
> immensely and making the project *much easier* to read and get used to.

With all respect, these plugins do not have as many contributors as Foreman 
and I'd like to hear from these contributors whether rubocop helped or annoyed 
them. I don't think that all cops enabled == good coding standards. And it 
does not imply good readability, that's on reviewer. TBH I think it's also 
pretty subjective (remember explicit return cop discussion).

--
Marek

On Friday 19 of August 2016 11:54:23 Daniel Lobato Garcia wrote:
> On 08/19, Lukas Zapletal wrote:
> > > As discussed on IRC yesterday there should be consistency and there is
> > > an
> > > option to autofix with rubocop if the style is changed to change
> > > existing
> > > code with less effort.
> > 
> > TL;DR - Let's keep Rubocop away from rockethash thing.
> > 
> > What the consistency gives us? We all know there are two ways and both
> > will work. Let's avoid big bangs that will make cherry picking harder
> > and just let's slowly improve as the time goes on.
> 
> Not sure what cherry-picking becomes harder after this change? It's just
> that people might have to rebase their PRs? That's a small price to pay
> considering code is read 1000x more often than written.
> 
> > I see no point in changing a single line of code from old to new syntax
> > just for that. We should only change it when changing logic.
> > 
> > Even if Rubocop is able to check only for changed lines, I won't like
> > that at all. I do not want to switch my brain between Smart Proxy and
> > Foreman Core codebases. Both ways should work and be accepted. Let's
> > only make the old syntax preferable when reviewing and that's it.
> 
> Precisely that's the painpoint, reviewers shouldn't have to pay
> attention to that. One of the points for having style guidelines is that
> the code looks and reads as if it had been written by one person.
> 
> Think of it from the POV of the ocassional contributor who is just
> confused about which syntax to use because the code mixes them both for
> no reason.
> 
> > I think we implemented Rubocop far beyond what's reasonable point. It
> > make sense for dangerous constructs, but not in this case (and few
> > others).
> 
> I'd argue the opposite, in foreman_docker or foreman_ansible I think
> rubocop (with *all* cops enabled) helped maintain good coding standards
> immensely and making the project *much easier* to read and get used to.
> 
> Again I think we're really bikeshedding when the purpose of a style
> guide is to stop it and make code look more homogeneous
> 
> > --
> > Later,
> > 
> >  Lukas #lzap Zapletal
> > 
> > --
> > 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+unsubscr...@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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: This is a digitally signed message part.

Re: [foreman-dev] Hash rocket syntax

[David Davis]

I don’t care whether or not we use rubocop to enforce a particular style.

However, if we don’t use rubocop to enforce a style, we should stay away
from enforcing a hash style by commenting in PRs. Those comments would be
too trivial and distracting to me.

David

On Fri, Aug 19, 2016 at 3:03 AM, Lukas Zapletal  wrote:

> > As discussed on IRC yesterday there should be consistency and there is an
> > option to autofix with rubocop if the style is changed to change existing
> > code with less effort.
>
> TL;DR - Let's keep Rubocop away from rockethash thing.
>
> What the consistency gives us? We all know there are two ways and both
> will work. Let's avoid big bangs that will make cherry picking harder
> and just let's slowly improve as the time goes on.
>
> I see no point in changing a single line of code from old to new syntax
> just for that. We should only change it when changing logic.
>
> Even if Rubocop is able to check only for changed lines, I won't like
> that at all. I do not want to switch my brain between Smart Proxy and
> Foreman Core codebases. Both ways should work and be accepted. Let's
> only make the old syntax preferable when reviewing and that's it.
>
> I think we implemented Rubocop far beyond what's reasonable point. It
> make sense for dangerous constructs, but not in this case (and few
> others).
>
> --
> Later,
>  Lukas #lzap Zapletal
>
> --
> 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+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [foreman-dev] Hash rocket syntax

[Tomas Strachota]

On 08/19/2016 11:54 AM, Daniel Lobato Garcia wrote:

On 08/19, Lukas Zapletal wrote:

As discussed on IRC yesterday there should be consistency and there is an
option to autofix with rubocop if the style is changed to change existing
code with less effort.


TL;DR - Let's keep Rubocop away from rockethash thing.

What the consistency gives us? We all know there are two ways and both
will work. Let's avoid big bangs that will make cherry picking harder
and just let's slowly improve as the time goes on.


Not sure what cherry-picking becomes harder after this change? It's just
that people might have to rebase their PRs? That's a small price to pay
considering code is read 1000x more often than written.


Rebasing open PRs is fine. I think that bigger issue is cherry-picking 
from devel to stable branches.





I see no point in changing a single line of code from old to new syntax
just for that. We should only change it when changing logic.

Even if Rubocop is able to check only for changed lines, I won't like
that at all. I do not want to switch my brain between Smart Proxy and
Foreman Core codebases. Both ways should work and be accepted. Let's
only make the old syntax preferable when reviewing and that's it.


Precisely that's the painpoint, reviewers shouldn't have to pay
attention to that. One of the points for having style guidelines is that
the code looks and reads as if it had been written by one person.

Think of it from the POV of the ocassional contributor who is just
confused about which syntax to use because the code mixes them both for
no reason.


I think we implemented Rubocop far beyond what's reasonable point. It
make sense for dangerous constructs, but not in this case (and few
others).


I'd argue the opposite, in foreman_docker or foreman_ansible I think
rubocop (with *all* cops enabled) helped maintain good coding standards
immensely and making the project *much easier* to read and get used to.

Again I think we're really bikeshedding when the purpose of a style
guide is to stop it and make code look more homogeneous



--
Later,
 Lukas #lzap Zapletal

--
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+unsubscr...@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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Re: [foreman-dev] Hash rocket syntax

[Daniel Lobato Garcia]

On 08/19, Lukas Zapletal wrote:
> > As discussed on IRC yesterday there should be consistency and there is an
> > option to autofix with rubocop if the style is changed to change existing
> > code with less effort.
>
> TL;DR - Let's keep Rubocop away from rockethash thing.
>
> What the consistency gives us? We all know there are two ways and both
> will work. Let's avoid big bangs that will make cherry picking harder
> and just let's slowly improve as the time goes on.

Not sure what cherry-picking becomes harder after this change? It's just
that people might have to rebase their PRs? That's a small price to pay
considering code is read 1000x more often than written.

> I see no point in changing a single line of code from old to new syntax
> just for that. We should only change it when changing logic.
>
> Even if Rubocop is able to check only for changed lines, I won't like
> that at all. I do not want to switch my brain between Smart Proxy and
> Foreman Core codebases. Both ways should work and be accepted. Let's
> only make the old syntax preferable when reviewing and that's it.

Precisely that's the painpoint, reviewers shouldn't have to pay
attention to that. One of the points for having style guidelines is that
the code looks and reads as if it had been written by one person.

Think of it from the POV of the ocassional contributor who is just
confused about which syntax to use because the code mixes them both for
no reason.

> I think we implemented Rubocop far beyond what's reasonable point. It
> make sense for dangerous constructs, but not in this case (and few
> others).

I'd argue the opposite, in foreman_docker or foreman_ansible I think
rubocop (with *all* cops enabled) helped maintain good coding standards
immensely and making the project *much easier* to read and get used to.

Again I think we're really bikeshedding when the purpose of a style
guide is to stop it and make code look more homogeneous

>
> --
> Later,
>  Lukas #lzap Zapletal
>
> --
> 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+unsubscr...@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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: PGP signature

Re: [foreman-dev] Hash rocket syntax

[Ivan Necas]

I would say: if we want to make large-scale changes that would make
sense, let's move away
from option hashes to keywords. This would justify the large-scale
changes in my eyes,
and the non-hash rocket syntax just falls more naturally in that case.
And Rubocop
can take a rest here.

I share the lzap's view on Rubocop, but if you've followed some of the
discussions
around that so far, a) you know that, b) you know how the discussions
usually end
(hint https://github.com/theforeman/foreman/pull/3617)

-- Ivan

On Fri, Aug 19, 2016 at 9:39 AM, Tomas Strachota  wrote:
> On 08/18/2016 03:16 PM, Ori Rabin wrote:
>>
>> Hello,
>>
>> The hash rocket syntax is still in use throughout the project.
>> New prs are sometimes submitted with the x: y syntax instead and then
>> asked to change.
>>
>> As discussed on IRC yesterday there should be consistency and there is
>> an option to autofix with rubocop if the style is changed to change
>> existing code with less effort.
>>
>> Since right now the lowest ruby version supported in core is 2.0 and
>> there are even discussions about dropping that, should this syntax change?
>
>
> I have no strong opinion about rocket vs. json syntax and I can live with
> both. I started to lean towards json style recently. There's one thing I'm
> afraid of though:
> the autofixing with rubocop, which will complicate cherry-picks. So I'd say
> let's start writing new code with json-like hashes and slowly iterate
> towards it rather then autofix the whole codebase.
>
>>
>> Ori
>>
>> --
>> 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+unsubscr...@googlegroups.com
>> .
>> For more options, visit https://groups.google.com/d/optout.
>
>
> --
> 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+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

-- 
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [foreman-dev] Hash rocket syntax

[Tomas Strachota]

On 08/18/2016 03:16 PM, Ori Rabin wrote:

Hello,

The hash rocket syntax is still in use throughout the project.
New prs are sometimes submitted with the x: y syntax instead and then
asked to change.

As discussed on IRC yesterday there should be consistency and there is
an option to autofix with rubocop if the style is changed to change
existing code with less effort.

Since right now the lowest ruby version supported in core is 2.0 and
there are even discussions about dropping that, should this syntax change?


I have no strong opinion about rocket vs. json syntax and I can live 
with both. I started to lean towards json style recently. There's one 
thing I'm afraid of though:
the autofixing with rubocop, which will complicate cherry-picks. So I'd 
say let's start writing new code with json-like hashes and slowly 
iterate towards it rather then autofix the whole codebase.




Ori

--
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+unsubscr...@googlegroups.com
.
For more options, visit https://groups.google.com/d/optout.


--
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [foreman-dev] Hash rocket syntax

[Lukas Zapletal]

> As discussed on IRC yesterday there should be consistency and there is an
> option to autofix with rubocop if the style is changed to change existing
> code with less effort.

TL;DR - Let's keep Rubocop away from rockethash thing.

What the consistency gives us? We all know there are two ways and both
will work. Let's avoid big bangs that will make cherry picking harder
and just let's slowly improve as the time goes on.

I see no point in changing a single line of code from old to new syntax
just for that. We should only change it when changing logic.

Even if Rubocop is able to check only for changed lines, I won't like
that at all. I do not want to switch my brain between Smart Proxy and
Foreman Core codebases. Both ways should work and be accepted. Let's
only make the old syntax preferable when reviewing and that's it.

I think we implemented Rubocop far beyond what's reasonable point. It
make sense for dangerous constructs, but not in this case (and few
others).

-- 
Later,
 Lukas #lzap Zapletal

-- 
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.