Rubocop cops in Foreman

In order to have a more consistent rubocop configuration across Foreman and
Katello, I’d like to bring some cops that Katello has disabled in its
rubocop configuration over to Foreman. These are cops that we’ve decided
are a little bit too strict.

Currently they are disabled in the rubocop todo file in foreman meaning
they are not being enforced but they could potentially be if someone
removes them from the todo file.

I’m hoping to get some feedback as to which ones people would like not to
be disabled. I’ll collect the feedback and then open a PR based on it. For
reference, here is our rubocop configuration in Katello:

<https://github.com/Katello/katello/blob/master/.rubocop.yml>
<https://github.com/Katello/katello/blob/master/.rubocop.yml>
https://github.com/Katello/katello/blob/master/.rubocop.yml

And here are the cops I’d like to disable:

Style/LeadingCommentSpace
<http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/LeadingCommentSpace>
Style/IfUnlessModifier
<http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/IfUnlessModifier>
Style/RescueModifier
<http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RescueModifier>
Style/AssignmentInCondition
<http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Lint/AssignmentInCondition>
Style/WhileUntilModifier
<http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WhileUntilModifier>
Style/AlignParameters
<http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/AlignParameters>
Style/ParenthesesAroundCondition
<http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/ParenthesesAroundCondition>
Style/DotPosition
<http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/DotPosition>
Style/Lambda
<http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Lambda>
Style/RedundantSelf
<http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RedundantSelf>
Style/RedundantReturn
<http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RedundantReturn>
Style/SpaceInsideHashLiteralBraces
<http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SpaceInsideHashLiteralBraces>
Style/SingleLineBlockParams
<http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SingleLineBlockParams>
Style/Next <http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/Next>
Style/FormatString
<http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/FormatString>
Style/GuardClause
<http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/GuardClause>
Style/StringLiterals
<http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/StringLiterals>
Style/WordArray
<http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WordArray>
Rails/ScopeArgs
<http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Rails/ScopeArgs>
Style/EachWithObject
<http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/EachWithObject>
Style/SymbolProc
<http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/SymbolProc>

Let me know if there are any questions. Thanks.

David

> From: "David Davis" <daviddavis@redhat.com>
> To: foreman-dev@googlegroups.com
> Sent: Tuesday, June 21, 2016 8:59:54 AM
> Subject: [foreman-dev] Rubocop cops in Foreman
>
> In order to have a more consistent rubocop configuration across Foreman and
> Katello, I’d like to bring some cops that Katello has disabled in its
> rubocop configuration over to Foreman. These are cops that we’ve decided
> are a little bit too strict.
>
> Currently they are disabled in the rubocop todo file in foreman meaning
> they are not being enforced but they could potentially be if someone
> removes them from the todo file.

It doesn't look like all of the ones below are disabled in foreman - e.g.
redundant return, and lambda style. And quite a few of them I like.

> I’m hoping to get some feedback as to which ones people would like not to
> be disabled. I’ll collect the feedback and then open a PR based on it. For
> reference, here is our rubocop configuration in Katello:
>
> <https://github.com/Katello/katello/blob/master/.rubocop.yml>
> <https://github.com/Katello/katello/blob/master/.rubocop.yml>
> https://github.com/Katello/katello/blob/master/.rubocop.yml
>
> And here are the cops I’d like to disable:
>
> Style/LeadingCommentSpace
> <http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/LeadingCommentSpace>
> Style/IfUnlessModifier
> <http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/IfUnlessModifier>
> Style/RescueModifier
> <http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RescueModifier>
> Style/AssignmentInCondition
> <http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Lint/AssignmentInCondition>

Why disable this? If you're doing assignment in a conditional, it should be wrapped
in parens to indicate your intention, e.g. if (foo = 'bar'). If you don't do that,
then I think rubocop should complain. This kind of bug could go unnoticed if the normal
case is for the if to evaluate as true.

> Style/WhileUntilModifier
> <http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WhileUntilModifier>
> Style/AlignParameters
> <http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/AlignParameters>
> Style/ParenthesesAroundCondition
> <http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/ParenthesesAroundCondition>
> Style/DotPosition
> <http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/DotPosition>
> Style/Lambda
> <http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Lambda>

I thought we agreed to standardize on stabby lambda everywhere

> Style/RedundantSelf
> <http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RedundantSelf>
> Style/RedundantReturn
> <http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RedundantReturn>

I like this cop

> Style/SpaceInsideHashLiteralBraces
> <http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SpaceInsideHashLiteralBraces>
> Style/SingleLineBlockParams
> <http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SingleLineBlockParams>
> Style/Next <http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/Next>
> Style/FormatString
> <http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/FormatString>
> Style/GuardClause
> <http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/GuardClause>
> Style/StringLiterals
> <http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/StringLiterals>

Ditto

··· ----- Original Message -----

Style/WordArray
http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WordArray
Rails/ScopeArgs
http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Rails/ScopeArgs
Style/EachWithObject
http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/EachWithObject
Style/SymbolProc
http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/SymbolProc

Let me know if there are any questions. Thanks.

David


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.

I tried to voice my comments here

··· On 06/21, David Davis wrote: > In order to have a more consistent rubocop configuration across Foreman and > Katello, I’d like to bring some cops that Katello has disabled in its > rubocop configuration over to Foreman. These are cops that we’ve decided > are a little bit too strict. > > Currently they are disabled in the rubocop todo file in foreman meaning > they are *not* being enforced but they could potentially be if someone > removes them from the todo file. > > I’m hoping to get some feedback as to which ones people would like *not* to > be disabled. I’ll collect the feedback and then open a PR based on it. For > reference, here is our rubocop configuration in Katello: > > > > https://github.com/Katello/katello/blob/master/.rubocop.yml > > And here are the cops I’d like to disable: > > Style/LeadingCommentSpace > > Style/IfUnlessModifier > > Style/RescueModifier > > Style/AssignmentInCondition > > Style/WhileUntilModifier > > Style/AlignParameters > > Style/ParenthesesAroundCondition > > Style/DotPosition > > Style/Lambda > > Style/RedundantSelf > > Style/RedundantReturn > > Style/SpaceInsideHashLiteralBraces > > Style/SingleLineBlockParams > > Style/Next > Style/FormatString > > Style/GuardClause > > Style/StringLiterals > > Style/WordArray > > Rails/ScopeArgs > > Style/EachWithObject > > Style/SymbolProc > > > Let me know if there are any questions. Thanks. > > David > > -- > 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

GPG: http://keys.gnupg.net/pks/lookup?op=get&search=0x7A92D6DD38D6DE30
Keybase: elobato (Daniel Lobato Garcia) | Keybase

David

>
>
> > From: "David Davis" <daviddavis@redhat.com>
> > To: foreman-dev@googlegroups.com
> > Sent: Tuesday, June 21, 2016 8:59:54 AM
> > Subject: [foreman-dev] Rubocop cops in Foreman
> >
> > In order to have a more consistent rubocop configuration across Foreman
> and
> > Katello, I’d like to bring some cops that Katello has disabled in its
> > rubocop configuration over to Foreman. These are cops that we’ve decided
> > are a little bit too strict.
> >
> > Currently they are disabled in the rubocop todo file in foreman meaning
> > they are not being enforced but they could potentially be if someone
> > removes them from the todo file.
>
> It doesn't look like all of the ones below are disabled in foreman - e.g.
> redundant return, and lambda style. And quite a few of them I like.
>
>
Yea, I guess overlooked some cops. Let me compile a list with the ones that
are enabled in Foreman but disabled in Katello. Then we can discuss if we
want to enable them in Katello or disable them in Foreman.

>
> > I’m hoping to get some feedback as to which ones people would like not
> to
> > be disabled. I’ll collect the feedback and then open a PR based on it.
> For
> > reference, here is our rubocop configuration in Katello:
> >
> > <https://github.com/Katello/katello/blob/master/.rubocop.yml>
> > <https://github.com/Katello/katello/blob/master/.rubocop.yml>
> > https://github.com/Katello/katello/blob/master/.rubocop.yml
> >
> > And here are the cops I’d like to disable:
> >
> > Style/LeadingCommentSpace
> > <
> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/LeadingCommentSpace
> >
> > Style/IfUnlessModifier
> > <
> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/IfUnlessModifier
> >
> > Style/RescueModifier
> > <
> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RescueModifier
> >
> > Style/AssignmentInCondition
> > <
> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Lint/AssignmentInCondition
> >
>
> Why disable this? If you're doing assignment in a conditional, it should
> be wrapped
> in parens to indicate your intention, e.g. if (foo = 'bar'). If you don't
> do that,
> then I think rubocop should complain. This kind of bug could go unnoticed
> if the normal
> case is for the if to evaluate as true.
>

Originally this cop didn’t allow any assignments in conditions (regardless
of whether you use parentheses or not). However, it looks like now they
allow it so I’ll enable this cop in Katello and Foreman (unless there are
objections).

>
> > Style/WhileUntilModifier
> > <
> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WhileUntilModifier
> >
> > Style/AlignParameters
> > <
> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/AlignParameters
> >
> > Style/ParenthesesAroundCondition
> > <
> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/ParenthesesAroundCondition
> >
> > Style/DotPosition
> > <
> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/DotPosition
> >
> > Style/Lambda
> > <http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Lambda
> >
>
>
> I thought we agreed to standardize on stabby lambda everywhere
>

I don’t remember seeing a discussion for this but I can leave it enabled
unless anyone objects.

>
>
> > Style/RedundantSelf
> > <
> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RedundantSelf
> >
> > Style/RedundantReturn
> > <
> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RedundantReturn
> >
>
> I like this cop
>
>
Based on the outcome of this previous discussion (
https://groups.google.com/forum/#!topic/foreman-dev/77H7AN0wX4g) I’ll leave
this as enabled and enable it in Katello.

> > Style/SpaceInsideHashLiteralBraces
> > <
> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SpaceInsideHashLiteralBraces
> >
> > Style/SingleLineBlockParams
> > <
> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SingleLineBlockParams
> >
> > Style/Next <http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/Next>
> > Style/FormatString
> > <
> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/FormatString
> >
> > Style/GuardClause
> > <
> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/GuardClause
> >
> > Style/StringLiterals
> > <
> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/StringLiterals
> >
>
> Ditto
>

I don’t have a huge preference on this but I will say that it is going to
be a HUGE pain to fix all the places that mix single and double quotes.
Also, I find it kind of a pain to turn string literals into strings that
can be interpolated.

··· On Tue, Jun 21, 2016 at 9:39 AM, Stephen Benjamin wrote: > ----- Original Message -----

Style/WordArray
<
http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WordArray

Rails/ScopeArgs
<
http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Rails/ScopeArgs

Style/EachWithObject
http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/EachWithObject
Style/SymbolProc
http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/SymbolProc

Let me know if there are any questions. Thanks.

David


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.


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.

> From: "David Davis" <daviddavis@redhat.com>
> To: foreman-dev@googlegroups.com
> Sent: Tuesday, June 21, 2016 9:58:21 AM
> Subject: Re: [foreman-dev] Rubocop cops in Foreman
>
> David
>
>
> >
> >
> > > From: "David Davis" <daviddavis@redhat.com>
> > > To: foreman-dev@googlegroups.com
> > > Sent: Tuesday, June 21, 2016 8:59:54 AM
> > > Subject: [foreman-dev] Rubocop cops in Foreman
> > >
> > > In order to have a more consistent rubocop configuration across Foreman
> > and
> > > Katello, I’d like to bring some cops that Katello has disabled in its
> > > rubocop configuration over to Foreman. These are cops that we’ve decided
> > > are a little bit too strict.
> > >
> > > Currently they are disabled in the rubocop todo file in foreman meaning
> > > they are not being enforced but they could potentially be if someone
> > > removes them from the todo file.
> >
> > It doesn't look like all of the ones below are disabled in foreman - e.g.
> > redundant return, and lambda style. And quite a few of them I like.
> >
> >
> Yea, I guess overlooked some cops. Let me compile a list with the ones that
> are enabled in Foreman but disabled in Katello. Then we can discuss if we
> want to enable them in Katello or disable them in Foreman.
>
>
> >
> > > I’m hoping to get some feedback as to which ones people would like not
> > to
> > > be disabled. I’ll collect the feedback and then open a PR based on it.
> > For
> > > reference, here is our rubocop configuration in Katello:
> > >
> > > <https://github.com/Katello/katello/blob/master/.rubocop.yml>
> > > <https://github.com/Katello/katello/blob/master/.rubocop.yml>
> > > https://github.com/Katello/katello/blob/master/.rubocop.yml
> > >
> > > And here are the cops I’d like to disable:
> > >
> > > Style/LeadingCommentSpace
> > > <
> > http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/LeadingCommentSpace
> > >
> > > Style/IfUnlessModifier
> > > <
> > http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/IfUnlessModifier
> > >
> > > Style/RescueModifier
> > > <
> > http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RescueModifier
> > >
> > > Style/AssignmentInCondition
> > > <
> > http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Lint/AssignmentInCondition
> > >
> >
> > Why disable this? If you're doing assignment in a conditional, it should
> > be wrapped
> > in parens to indicate your intention, e.g. if (foo = 'bar'). If you don't
> > do that,
> > then I think rubocop should complain. This kind of bug could go unnoticed
> > if the normal
> > case is for the if to evaluate as true.
> >
>
> Originally this cop didn’t allow any assignments in conditions (regardless
> of whether you use parentheses or not). However, it looks like now they
> allow it so I’ll enable this cop in Katello and Foreman (unless there are
> objections).
>
>
> >
> > > Style/WhileUntilModifier
> > > <
> > http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WhileUntilModifier
> > >
> > > Style/AlignParameters
> > > <
> > http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/AlignParameters
> > >
> > > Style/ParenthesesAroundCondition
> > > <
> > http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/ParenthesesAroundCondition
> > >
> > > Style/DotPosition
> > > <
> > http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/DotPosition
> > >
> > > Style/Lambda
> > > <http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Lambda
> > >
> >
> >
> > I thought we agreed to standardize on stabby lambda everywhere
> >
>
> I don’t remember seeing a discussion for this but I can leave it enabled
> unless anyone objects.

I guess it wasn't really discussed on the list, the cop was just explicitly
enabled and the one-liners all moved to the stabby type: https://github.com/theforeman/foreman/pull/2605

> >
> >
> > > Style/RedundantSelf
> > > <
> > http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RedundantSelf
> > >
> > > Style/RedundantReturn
> > > <
> > http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RedundantReturn
> > >
> >
> > I like this cop
> >
> >
> Based on the outcome of this previous discussion (
> https://groups.google.com/forum/#!topic/foreman-dev/77H7AN0wX4g) I’ll leave
> this as enabled and enable it in Katello.
>
>
> > > Style/SpaceInsideHashLiteralBraces
> > > <
> > http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SpaceInsideHashLiteralBraces
> > >
> > > Style/SingleLineBlockParams
> > > <
> > http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SingleLineBlockParams
> > >
> > > Style/Next <http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/Next>
> > > Style/FormatString
> > > <
> > http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/FormatString
> > >
> > > Style/GuardClause
> > > <
> > http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/GuardClause
> > >
> > > Style/StringLiterals
> > > <
> > http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/StringLiterals
> > >
> >
> > Ditto
> >
>
> I don’t have a huge preference on this but I will say that it is going to
> be a HUGE pain to fix all the places that mix single and double quotes.
> Also, I find it kind of a pain to turn string literals into strings that
> can be interpolated.

Ok, fair enough, I do agree it's a pain. For some reason I thought using single
quotes was faster since it didn't have to look for interpolation but the internets
seem to say there's no significant difference.

··· ----- Original Message ----- > On Tue, Jun 21, 2016 at 9:39 AM, Stephen Benjamin > wrote: > > ----- Original Message -----

Style/WordArray
<
http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WordArray

Rails/ScopeArgs
<
http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Rails/ScopeArgs

Style/EachWithObject
http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/EachWithObject
Style/SymbolProc
http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/SymbolProc

Let me know if there are any questions. Thanks.

David


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.


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.


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.

What about doing a poll for voting for the cops you're suggesting to
disable? I'm afraid
the mail thread is not a suitable format for this kind of questions.

– Ivan

··· On Tue, Jun 21, 2016 at 6:03 PM, Stephen Benjamin wrote:

----- Original Message -----

From: “David Davis” daviddavis@redhat.com
To: foreman-dev@googlegroups.com
Sent: Tuesday, June 21, 2016 9:58:21 AM
Subject: Re: [foreman-dev] Rubocop cops in Foreman

David

On Tue, Jun 21, 2016 at 9:39 AM, Stephen Benjamin stephen@redhat.com > > wrote:

----- Original Message -----

From: “David Davis” daviddavis@redhat.com
To: foreman-dev@googlegroups.com
Sent: Tuesday, June 21, 2016 8:59:54 AM
Subject: [foreman-dev] Rubocop cops in Foreman

In order to have a more consistent rubocop configuration across
Foreman

and

Katello, I’d like to bring some cops that Katello has disabled in its
rubocop configuration over to Foreman. These are cops that we’ve
decided

are a little bit too strict.

Currently they are disabled in the rubocop todo file in foreman
meaning

they are not being enforced but they could potentially be if
someone

removes them from the todo file.

It doesn’t look like all of the ones below are disabled in foreman -
e.g.

redundant return, and lambda style. And quite a few of them I like.

Yea, I guess overlooked some cops. Let me compile a list with the ones
that
are enabled in Foreman but disabled in Katello. Then we can discuss if we
want to enable them in Katello or disable them in Foreman.

I’m hoping to get some feedback as to which ones people would like
not

to

be disabled. I’ll collect the feedback and then open a PR based on
it.

For

reference, here is our rubocop configuration in Katello:

https://github.com/Katello/katello/blob/master/.rubocop.yml
https://github.com/Katello/katello/blob/master/.rubocop.yml
https://github.com/Katello/katello/blob/master/.rubocop.yml

And here are the cops I’d like to disable:

Style/LeadingCommentSpace
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/LeadingCommentSpace

Style/IfUnlessModifier
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/IfUnlessModifier

Style/RescueModifier
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RescueModifier

Style/AssignmentInCondition
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Lint/AssignmentInCondition

Why disable this? If you’re doing assignment in a conditional, it
should

be wrapped
in parens to indicate your intention, e.g. if (foo = ‘bar’). If you
don’t

do that,
then I think rubocop should complain. This kind of bug could go
unnoticed

if the normal
case is for the if to evaluate as true.

Originally this cop didn’t allow any assignments in conditions
(regardless
of whether you use parentheses or not). However, it looks like now they
allow it so I’ll enable this cop in Katello and Foreman (unless there are
objections).

Style/WhileUntilModifier
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WhileUntilModifier

Style/AlignParameters
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/AlignParameters

Style/ParenthesesAroundCondition
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/ParenthesesAroundCondition

Style/DotPosition
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/DotPosition

Style/Lambda
<
http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Lambda

I thought we agreed to standardize on stabby lambda everywhere

I don’t remember seeing a discussion for this but I can leave it enabled
unless anyone objects.

I guess it wasn’t really discussed on the list, the cop was just explicitly
enabled and the one-liners all moved to the stabby type:
https://github.com/theforeman/foreman/pull/2605

Style/RedundantSelf
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RedundantSelf

Style/RedundantReturn
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RedundantReturn

I like this cop

Based on the outcome of this previous discussion (
https://groups.google.com/forum/#!topic/foreman-dev/77H7AN0wX4g) I’ll
leave
this as enabled and enable it in Katello.

Style/SpaceInsideHashLiteralBraces
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SpaceInsideHashLiteralBraces

Style/SingleLineBlockParams
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SingleLineBlockParams

Style/Next <
http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/Next>

Style/FormatString
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/FormatString

Style/GuardClause
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/GuardClause

Style/StringLiterals
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/StringLiterals

Ditto

I don’t have a huge preference on this but I will say that it is going to
be a HUGE pain to fix all the places that mix single and double quotes.
Also, I find it kind of a pain to turn string literals into strings that
can be interpolated.

Ok, fair enough, I do agree it’s a pain. For some reason I thought using
single
quotes was faster since it didn’t have to look for interpolation but the
internets
seem to say there’s no significant difference.

Style/WordArray
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WordArray

Rails/ScopeArgs
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Rails/ScopeArgs

Style/EachWithObject
<
http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/EachWithObject>

Style/SymbolProc
http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/SymbolProc

Let me know if there are any questions. Thanks.

David


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.


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.


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.


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.

I can do a poll. I actually thought about doing it before but since I am
not planning on actually enabling any of these cops the options would be
something like “disable” or “do nothing”. Would that make sense to people?

David

··· On Wed, Jun 22, 2016 at 4:50 AM, Ivan Necas wrote:

What about doing a poll for voting for the cops you’re suggesting to
disable? I’m afraid
the mail thread is not a suitable format for this kind of questions.

– Ivan

On Tue, Jun 21, 2016 at 6:03 PM, Stephen Benjamin stephen@redhat.com > wrote:

----- Original Message -----

From: “David Davis” daviddavis@redhat.com
To: foreman-dev@googlegroups.com
Sent: Tuesday, June 21, 2016 9:58:21 AM
Subject: Re: [foreman-dev] Rubocop cops in Foreman

David

On Tue, Jun 21, 2016 at 9:39 AM, Stephen Benjamin stephen@redhat.com >> > wrote:

----- Original Message -----

From: “David Davis” daviddavis@redhat.com
To: foreman-dev@googlegroups.com
Sent: Tuesday, June 21, 2016 8:59:54 AM
Subject: [foreman-dev] Rubocop cops in Foreman

In order to have a more consistent rubocop configuration across
Foreman

and

Katello, I’d like to bring some cops that Katello has disabled in
its

rubocop configuration over to Foreman. These are cops that we’ve
decided

are a little bit too strict.

Currently they are disabled in the rubocop todo file in foreman
meaning

they are not being enforced but they could potentially be if
someone

removes them from the todo file.

It doesn’t look like all of the ones below are disabled in foreman -
e.g.

redundant return, and lambda style. And quite a few of them I like.

Yea, I guess overlooked some cops. Let me compile a list with the ones
that
are enabled in Foreman but disabled in Katello. Then we can discuss if
we
want to enable them in Katello or disable them in Foreman.

I’m hoping to get some feedback as to which ones people would like
not

to

be disabled. I’ll collect the feedback and then open a PR based on
it.

For

reference, here is our rubocop configuration in Katello:

https://github.com/Katello/katello/blob/master/.rubocop.yml
https://github.com/Katello/katello/blob/master/.rubocop.yml
https://github.com/Katello/katello/blob/master/.rubocop.yml

And here are the cops I’d like to disable:

Style/LeadingCommentSpace
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/LeadingCommentSpace

Style/IfUnlessModifier
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/IfUnlessModifier

Style/RescueModifier
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RescueModifier

Style/AssignmentInCondition
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Lint/AssignmentInCondition

Why disable this? If you’re doing assignment in a conditional, it
should

be wrapped
in parens to indicate your intention, e.g. if (foo = ‘bar’). If you
don’t

do that,
then I think rubocop should complain. This kind of bug could go
unnoticed

if the normal
case is for the if to evaluate as true.

Originally this cop didn’t allow any assignments in conditions
(regardless
of whether you use parentheses or not). However, it looks like now they
allow it so I’ll enable this cop in Katello and Foreman (unless there
are
objections).

Style/WhileUntilModifier
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WhileUntilModifier

Style/AlignParameters
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/AlignParameters

Style/ParenthesesAroundCondition
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/ParenthesesAroundCondition

Style/DotPosition
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/DotPosition

Style/Lambda
<
http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Lambda

I thought we agreed to standardize on stabby lambda everywhere

I don’t remember seeing a discussion for this but I can leave it enabled
unless anyone objects.

I guess it wasn’t really discussed on the list, the cop was just
explicitly
enabled and the one-liners all moved to the stabby type:
https://github.com/theforeman/foreman/pull/2605

Style/RedundantSelf
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RedundantSelf

Style/RedundantReturn
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RedundantReturn

I like this cop

Based on the outcome of this previous discussion (
https://groups.google.com/forum/#!topic/foreman-dev/77H7AN0wX4g) I’ll
leave
this as enabled and enable it in Katello.

Style/SpaceInsideHashLiteralBraces
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SpaceInsideHashLiteralBraces

Style/SingleLineBlockParams
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SingleLineBlockParams

Style/Next <
http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/Next>

Style/FormatString
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/FormatString

Style/GuardClause
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/GuardClause

Style/StringLiterals
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/StringLiterals

Ditto

I don’t have a huge preference on this but I will say that it is going
to
be a HUGE pain to fix all the places that mix single and double quotes.
Also, I find it kind of a pain to turn string literals into strings that
can be interpolated.

Ok, fair enough, I do agree it’s a pain. For some reason I thought using
single
quotes was faster since it didn’t have to look for interpolation but the
internets
seem to say there’s no significant difference.

Style/WordArray
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WordArray

Rails/ScopeArgs
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Rails/ScopeArgs

Style/EachWithObject
<
http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/EachWithObject>

Style/SymbolProc
http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/SymbolProc

Let me know if there are any questions. Thanks.

David


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.


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.


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.


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.


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.

Ok, created a poll for the cops that are disabled in Katello but are
outstanding in Foreman. I’ll create one PR for the ones we want to disable
and then probably follow up with individual PRs for the ones we want
enabled.

http://goo.gl/forms/XQWiGlqrvof8yuWP2

David

··· On Wed, Jun 22, 2016 at 8:24 AM, David Davis wrote:

I can do a poll. I actually thought about doing it before but since I am
not planning on actually enabling any of these cops the options would be
something like “disable” or “do nothing”. Would that make sense to people?

David

On Wed, Jun 22, 2016 at 4:50 AM, Ivan Necas inecas@redhat.com wrote:

What about doing a poll for voting for the cops you’re suggesting to
disable? I’m afraid
the mail thread is not a suitable format for this kind of questions.

– Ivan

On Tue, Jun 21, 2016 at 6:03 PM, Stephen Benjamin stephen@redhat.com >> wrote:

----- Original Message -----

From: “David Davis” daviddavis@redhat.com
To: foreman-dev@googlegroups.com
Sent: Tuesday, June 21, 2016 9:58:21 AM
Subject: Re: [foreman-dev] Rubocop cops in Foreman

David

On Tue, Jun 21, 2016 at 9:39 AM, Stephen Benjamin stephen@redhat.com >>> > wrote:

----- Original Message -----

From: “David Davis” daviddavis@redhat.com
To: foreman-dev@googlegroups.com
Sent: Tuesday, June 21, 2016 8:59:54 AM
Subject: [foreman-dev] Rubocop cops in Foreman

In order to have a more consistent rubocop configuration across
Foreman
and
Katello, I’d like to bring some cops that Katello has disabled in
its
rubocop configuration over to Foreman. These are cops that we’ve
decided
are a little bit too strict.

Currently they are disabled in the rubocop todo file in foreman
meaning
they are not being enforced but they could potentially be if
someone
removes them from the todo file.

It doesn’t look like all of the ones below are disabled in foreman -
e.g.
redundant return, and lambda style. And quite a few of them I like.

Yea, I guess overlooked some cops. Let me compile a list with the ones
that
are enabled in Foreman but disabled in Katello. Then we can discuss if
we
want to enable them in Katello or disable them in Foreman.

I’m hoping to get some feedback as to which ones people would like
not
to
be disabled. I’ll collect the feedback and then open a PR based on
it.
For
reference, here is our rubocop configuration in Katello:

https://github.com/Katello/katello/blob/master/.rubocop.yml
https://github.com/Katello/katello/blob/master/.rubocop.yml
https://github.com/Katello/katello/blob/master/.rubocop.yml

And here are the cops I’d like to disable:

Style/LeadingCommentSpace
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/LeadingCommentSpace

Style/IfUnlessModifier
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/IfUnlessModifier

Style/RescueModifier
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RescueModifier

Style/AssignmentInCondition
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Lint/AssignmentInCondition

Why disable this? If you’re doing assignment in a conditional, it
should
be wrapped
in parens to indicate your intention, e.g. if (foo = ‘bar’). If you
don’t
do that,
then I think rubocop should complain. This kind of bug could go
unnoticed
if the normal
case is for the if to evaluate as true.

Originally this cop didn’t allow any assignments in conditions
(regardless
of whether you use parentheses or not). However, it looks like now they
allow it so I’ll enable this cop in Katello and Foreman (unless there
are
objections).

Style/WhileUntilModifier
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WhileUntilModifier

Style/AlignParameters
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/AlignParameters

Style/ParenthesesAroundCondition
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/ParenthesesAroundCondition

Style/DotPosition
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/DotPosition

Style/Lambda
<
http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Lambda

I thought we agreed to standardize on stabby lambda everywhere

I don’t remember seeing a discussion for this but I can leave it
enabled
unless anyone objects.

I guess it wasn’t really discussed on the list, the cop was just
explicitly
enabled and the one-liners all moved to the stabby type:
https://github.com/theforeman/foreman/pull/2605

Style/RedundantSelf
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RedundantSelf

Style/RedundantReturn
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RedundantReturn

I like this cop

Based on the outcome of this previous discussion (
Redirecting to Google Groups) I’ll
leave
this as enabled and enable it in Katello.

Style/SpaceInsideHashLiteralBraces
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SpaceInsideHashLiteralBraces

Style/SingleLineBlockParams
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SingleLineBlockParams

Style/Next <
Class: RuboCop::Cop::Style::Next — Documentation for rubocop (1.56.4)>
Style/FormatString
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/FormatString

Style/GuardClause
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/GuardClause

Style/StringLiterals
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/StringLiterals

Ditto

I don’t have a huge preference on this but I will say that it is going
to
be a HUGE pain to fix all the places that mix single and double quotes.
Also, I find it kind of a pain to turn string literals into strings
that
can be interpolated.

Ok, fair enough, I do agree it’s a pain. For some reason I thought
using single
quotes was faster since it didn’t have to look for interpolation but the
internets
seem to say there’s no significant difference.

Style/WordArray
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WordArray

Rails/ScopeArgs
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Rails/ScopeArgs

Style/EachWithObject
<
Class: RuboCop::Cop::Style::EachWithObject — Documentation for rubocop (1.56.4)>
Style/SymbolProc
<Class: RuboCop::Cop::Style::SymbolProc — Documentation for rubocop (1.56.4)

Let me know if there are any questions. Thanks.

David


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.


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.


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.


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.


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.

Thank you very much for this effort David.

··· -- Marek

On Wednesday 22 of June 2016 10:33:53 David Davis wrote:

Ok, created a poll for the cops that are disabled in Katello but are
outstanding in Foreman. I’ll create one PR for the ones we want to disable
and then probably follow up with individual PRs for the ones we want
enabled.

http://goo.gl/forms/XQWiGlqrvof8yuWP2

David

On Wed, Jun 22, 2016 at 8:24 AM, David Davis daviddavis@redhat.com wrote:

I can do a poll. I actually thought about doing it before but since I am
not planning on actually enabling any of these cops the options would be
something like “disable” or “do nothing”. Would that make sense to people?

David

On Wed, Jun 22, 2016 at 4:50 AM, Ivan Necas inecas@redhat.com wrote:

What about doing a poll for voting for the cops you’re suggesting to
disable? I’m afraid
the mail thread is not a suitable format for this kind of questions.

– Ivan

On Tue, Jun 21, 2016 at 6:03 PM, Stephen Benjamin stephen@redhat.com > >> > >> wrote:

----- Original Message -----

From: “David Davis” daviddavis@redhat.com
To: foreman-dev@googlegroups.com
Sent: Tuesday, June 21, 2016 9:58:21 AM
Subject: Re: [foreman-dev] Rubocop cops in Foreman

David

On Tue, Jun 21, 2016 at 9:39 AM, Stephen Benjamin stephen@redhat.com > >>> > > >>> > wrote:

----- Original Message -----

From: “David Davis” daviddavis@redhat.com
To: foreman-dev@googlegroups.com
Sent: Tuesday, June 21, 2016 8:59:54 AM
Subject: [foreman-dev] Rubocop cops in Foreman

In order to have a more consistent rubocop configuration across

Foreman

and

Katello, I’d like to bring some cops that Katello has disabled in

its

rubocop configuration over to Foreman. These are cops that we’ve

decided

are a little bit too strict.

Currently they are disabled in the rubocop todo file in foreman

meaning

they are not being enforced but they could potentially be if

someone

removes them from the todo file.

It doesn’t look like all of the ones below are disabled in foreman -

e.g.

redundant return, and lambda style. And quite a few of them I like.

Yea, I guess overlooked some cops. Let me compile a list with the ones

that

are enabled in Foreman but disabled in Katello. Then we can discuss if

we

want to enable them in Katello or disable them in Foreman.

I’m hoping to get some feedback as to which ones people would like

not

to

be disabled. I’ll collect the feedback and then open a PR based on

it.

For

reference, here is our rubocop configuration in Katello:

https://github.com/Katello/katello/blob/master/.rubocop.yml
https://github.com/Katello/katello/blob/master/.rubocop.yml
https://github.com/Katello/katello/blob/master/.rubocop.yml

And here are the cops I’d like to disable:

Style/LeadingCommentSpace
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Leading

Style/IfUnlessModifier
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/IfUnles

Style/RescueModifier
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RescueM

Style/AssignmentInCondition
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Lint/Assignme

Why disable this? If you’re doing assignment in a conditional, it

should

be wrapped
in parens to indicate your intention, e.g. if (foo = ‘bar’). If you

don’t

do that,
then I think rubocop should complain. This kind of bug could go

unnoticed

if the normal
case is for the if to evaluate as true.

Originally this cop didn’t allow any assignments in conditions

(regardless

of whether you use parentheses or not). However, it looks like now
they
allow it so I’ll enable this cop in Katello and Foreman (unless there

are

objections).

Style/WhileUntilModifier
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WhileUn

Style/AlignParameters
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/AlignPa

Style/ParenthesesAroundCondition
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Parenth

Style/DotPosition
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/DotPosi

Style/Lambda
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Lambda

I thought we agreed to standardize on stabby lambda everywhere

I don’t remember seeing a discussion for this but I can leave it

enabled

unless anyone objects.

I guess it wasn’t really discussed on the list, the cop was just
explicitly
enabled and the one-liners all moved to the stabby type:
https://github.com/theforeman/foreman/pull/2605

Style/RedundantSelf
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Redunda

Style/RedundantReturn
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Redunda

I like this cop

Based on the outcome of this previous discussion (
https://groups.google.com/forum/#!topic/foreman-dev/77H7AN0wX4g) I’ll

leave

this as enabled and enable it in Katello.

Style/SpaceInsideHashLiteralBraces
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SpaceIn

Style/SingleLineBlockParams
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SingleL

Style/Next <

http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/Next>

Style/FormatString
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/FormatS

Style/GuardClause
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/GuardCl

Style/StringLiterals
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/StringL

Ditto

I don’t have a huge preference on this but I will say that it is going

to

be a HUGE pain to fix all the places that mix single and double
quotes.
Also, I find it kind of a pain to turn string literals into strings

that

can be interpolated.

Ok, fair enough, I do agree it’s a pain. For some reason I thought
using single
quotes was faster since it didn’t have to look for interpolation but the
internets
seem to say there’s no significant difference.

Style/WordArray
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WordArr
ay

Rails/ScopeArgs
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Rails/ScopeAr
gs

Style/EachWithObject
<

http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/EachWithObject>

Style/SymbolProc
<http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/SymbolProc

Let me know if there are any questions. Thanks.

David


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.


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.


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.


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.


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.

Thanks David for the poll. It made me realize how terrible the code
would be if all the cops in rubocop were enabled :slight_smile:

– Ivan

··· On Thu, Jun 23, 2016 at 10:10 AM, Marek Hulán wrote: > Thank you very much for this effort David. > > -- > Marek > > On Wednesday 22 of June 2016 10:33:53 David Davis wrote: >> Ok, created a poll for the cops that are disabled in Katello but are >> outstanding in Foreman. I’ll create one PR for the ones we want to disable >> and then probably follow up with individual PRs for the ones we want >> enabled. >> >> http://goo.gl/forms/XQWiGlqrvof8yuWP2 >> >> >> David >> >> On Wed, Jun 22, 2016 at 8:24 AM, David Davis wrote: >> > I can do a poll. I actually thought about doing it before but since I am >> > not planning on actually enabling any of these cops the options would be >> > something like “disable” or “do nothing”. Would that make sense to people? >> > >> > >> > David >> > >> > On Wed, Jun 22, 2016 at 4:50 AM, Ivan Necas wrote: >> >> What about doing a poll for voting for the cops you're suggesting to >> >> disable? I'm afraid >> >> the mail thread is not a suitable format for this kind of questions. >> >> >> >> -- Ivan >> >> >> >> On Tue, Jun 21, 2016 at 6:03 PM, Stephen Benjamin >> >> >> >> wrote: >> >>> ----- Original Message ----- >> >>> >> >>> > From: "David Davis" >> >>> > To: foreman-dev@googlegroups.com >> >>> > Sent: Tuesday, June 21, 2016 9:58:21 AM >> >>> > Subject: Re: [foreman-dev] Rubocop cops in Foreman >> >>> > >> >>> > David >> >>> > >> >>> > On Tue, Jun 21, 2016 at 9:39 AM, Stephen Benjamin >> >>> > >> >>> > wrote: >> >>> > > ----- Original Message ----- >> >>> > > >> >>> > > > From: "David Davis" >> >>> > > > To: foreman-dev@googlegroups.com >> >>> > > > Sent: Tuesday, June 21, 2016 8:59:54 AM >> >>> > > > Subject: [foreman-dev] Rubocop cops in Foreman >> >>> > > > >> >>> > > > In order to have a more consistent rubocop configuration across >> >>> >> >>> Foreman >> >>> >> >>> > > and >> >>> > > >> >>> > > > Katello, I’d like to bring some cops that Katello has disabled in >> >>> >> >>> its >> >>> >> >>> > > > rubocop configuration over to Foreman. These are cops that we’ve >> >>> >> >>> decided >> >>> >> >>> > > > are a little bit too strict. >> >>> > > > >> >>> > > > Currently they are disabled in the rubocop todo file in foreman >> >>> >> >>> meaning >> >>> >> >>> > > > they are *not* being enforced but they could potentially be if >> >>> >> >>> someone >> >>> >> >>> > > > removes them from the todo file. >> >>> > > >> >>> > > It doesn't look like all of the ones below are disabled in foreman - >> >>> >> >>> e.g. >> >>> >> >>> > > redundant return, and lambda style. And quite a few of them I like. >> >>> > >> >>> > Yea, I guess overlooked some cops. Let me compile a list with the ones >> >>> >> >>> that >> >>> >> >>> > are enabled in Foreman but disabled in Katello. Then we can discuss if >> >>> >> >>> we >> >>> >> >>> > want to enable them in Katello or disable them in Foreman. >> >>> > >> >>> > > > I’m hoping to get some feedback as to which ones people would like >> >>> >> >>> *not* >> >>> >> >>> > > to >> >>> > > >> >>> > > > be disabled. I’ll collect the feedback and then open a PR based on >> >>> >> >>> it. >> >>> >> >>> > > For >> >>> > > >> >>> > > > reference, here is our rubocop configuration in Katello: >> >>> > > > >> >>> > > > >> >>> > > > >> >>> > > > https://github.com/Katello/katello/blob/master/.rubocop.yml >> >>> > > > >> >>> > > > And here are the cops I’d like to disable: >> >>> > > > >> >>> > > > Style/LeadingCommentSpace >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Leading >> >>> >>> >> >>> > > > Style/IfUnlessModifier >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/IfUnles >> >>> >>> >> >>> > > > Style/RescueModifier >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RescueM >> >>> >>> >> >>> > > > Style/AssignmentInCondition >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Lint/Assignme >> >>> >>> >> >>> > > Why disable this? If you're doing assignment in a conditional, it >> >>> >> >>> should >> >>> >> >>> > > be wrapped >> >>> > > in parens to indicate your intention, e.g. if (foo = 'bar'). If you >> >>> >> >>> don't >> >>> >> >>> > > do that, >> >>> > > then I think rubocop should complain. This kind of bug could go >> >>> >> >>> unnoticed >> >>> >> >>> > > if the normal >> >>> > > case is for the `if` to evaluate as true. >> >>> > >> >>> > Originally this cop didn’t allow any assignments in conditions >> >>> >> >>> (regardless >> >>> >> >>> > of whether you use parentheses or not). However, it looks like now >> >>> > they >> >>> > allow it so I’ll enable this cop in Katello and Foreman (unless there >> >>> >> >>> are >> >>> >> >>> > objections). >> >>> > >> >>> > > > Style/WhileUntilModifier >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WhileUn >> >>> >>> >> >>> > > > Style/AlignParameters >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/AlignPa >> >>> >>> >> >>> > > > Style/ParenthesesAroundCondition >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Parenth >> >>> >>> >> >>> > > > Style/DotPosition >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/DotPosi >> >>> >>> >> >>> > > > Style/Lambda >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Lambda >> >>> >> >>> > > I thought we agreed to standardize on stabby lambda everywhere >> >>> > >> >>> > I don’t remember seeing a discussion for this but I can leave it >> >>> >> >>> enabled >> >>> >> >>> > unless anyone objects. >> >>> >> >>> I guess it wasn't really discussed on the list, the cop was just >> >>> explicitly >> >>> enabled and the one-liners all moved to the stabby type: >> >>> https://github.com/theforeman/foreman/pull/2605 >> >>> >> >>> > > > Style/RedundantSelf >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Redunda >> >>> >>> >> >>> > > > Style/RedundantReturn >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Redunda >> >>> >>> >> >>> > > I like this cop >> >>> > >> >>> > Based on the outcome of this previous discussion ( >> >>> > https://groups.google.com/forum/#!topic/foreman-dev/77H7AN0wX4g) I’ll >> >>> >> >>> leave >> >>> >> >>> > this as enabled and enable it in Katello. >> >>> > >> >>> > > > Style/SpaceInsideHashLiteralBraces >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SpaceIn >> >>> >>> >> >>> > > > Style/SingleLineBlockParams >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SingleL >> >>> >>> >> >>> > > > Style/Next < >> >>> >> >>> http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/Next> >> >>> >> >>> > > > Style/FormatString >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/FormatS >> >>> >>> >> >>> > > > Style/GuardClause >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/GuardCl >> >>> >>> >> >>> > > > Style/StringLiterals >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/StringL >> >>> >>> >> >>> > > Ditto >> >>> > >> >>> > I don’t have a huge preference on this but I will say that it is going >> >>> >> >>> to >> >>> >> >>> > be a HUGE pain to fix all the places that mix single and double >> >>> > quotes. >> >>> > Also, I find it kind of a pain to turn string literals into strings >> >>> >> >>> that >> >>> >> >>> > can be interpolated. >> >>> >> >>> Ok, fair enough, I do agree it's a pain. For some reason I thought >> >>> using single >> >>> quotes was faster since it didn't have to look for interpolation but the >> >>> internets >> >>> seem to say there's no significant difference. >> >>> >> >>> > > > Style/WordArray >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WordArr >> >>> ay >> >>> >> >>> > > > Rails/ScopeArgs >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Rails/ScopeAr >> >>> gs >> >>> >> >>> > > > Style/EachWithObject >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/EachWithObject> >> >>> >> >>> > > > Style/SymbolProc >> >>> > > > > >>> > > > >> >>> > > > >> >>> > > > Let me know if there are any questions. Thanks. >> >>> > > > >> >>> > > > David >> >>> > > > >> >>> > > > -- >> >>> > > > 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. >> >>> > > >> >>> > > -- >> >>> > > 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. >> >>> > >> >>> > -- >> >>> > 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. >> >>> >> >>> -- >> >>> 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. >> >> >> >> -- >> >> 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. > > -- > 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.

I don't think the purpose of Rubocop or the style guide is really
fulfilled if we start debating about most of them.

  1. The purpose of the style guide is precisely that we don't argue
    anymore about this (bikeshedding, really) and we just stick to good
    practices (approved by a LOT of companies & individuals, including
    GitHub and Shopify which are 2 of the largest Rails projects out there)

  2. 99% of cops are there for a good reason, including the method size,
    complexity ones, etc… if you feel strongly against any of these and
    you really have solid reasons, https://github.com/bbatsov/ruby-style-guide/issues
    is the place.

Having or not having a style-guide is an important decision,
debating every other rule is a triviality & detracts from its purpose.

··· On 06/28, Ivan Necas wrote: > Thanks David for the poll. It made me realize how terrible the code > would be if all the cops in rubocop were enabled :) > > -- Ivan > > On Thu, Jun 23, 2016 at 10:10 AM, Marek Hulán wrote: > > Thank you very much for this effort David. > > > > -- > > Marek > > > > On Wednesday 22 of June 2016 10:33:53 David Davis wrote: > >> Ok, created a poll for the cops that are disabled in Katello but are > >> outstanding in Foreman. I’ll create one PR for the ones we want to disable > >> and then probably follow up with individual PRs for the ones we want > >> enabled. > >> > >> http://goo.gl/forms/XQWiGlqrvof8yuWP2 > >> > >> > >> David > >> > >> On Wed, Jun 22, 2016 at 8:24 AM, David Davis wrote: > >> > I can do a poll. I actually thought about doing it before but since I am > >> > not planning on actually enabling any of these cops the options would be > >> > something like “disable” or “do nothing”. Would that make sense to people? > >> > > >> > > >> > David > >> > > >> > On Wed, Jun 22, 2016 at 4:50 AM, Ivan Necas wrote: > >> >> What about doing a poll for voting for the cops you're suggesting to > >> >> disable? I'm afraid > >> >> the mail thread is not a suitable format for this kind of questions. > >> >> > >> >> -- Ivan > >> >> > >> >> On Tue, Jun 21, 2016 at 6:03 PM, Stephen Benjamin > >> >> > >> >> wrote: > >> >>> ----- Original Message ----- > >> >>> > >> >>> > From: "David Davis" > >> >>> > To: foreman-dev@googlegroups.com > >> >>> > Sent: Tuesday, June 21, 2016 9:58:21 AM > >> >>> > Subject: Re: [foreman-dev] Rubocop cops in Foreman > >> >>> > > >> >>> > David > >> >>> > > >> >>> > On Tue, Jun 21, 2016 at 9:39 AM, Stephen Benjamin > >> >>> > > >> >>> > wrote: > >> >>> > > ----- Original Message ----- > >> >>> > > > >> >>> > > > From: "David Davis" > >> >>> > > > To: foreman-dev@googlegroups.com > >> >>> > > > Sent: Tuesday, June 21, 2016 8:59:54 AM > >> >>> > > > Subject: [foreman-dev] Rubocop cops in Foreman > >> >>> > > > > >> >>> > > > In order to have a more consistent rubocop configuration across > >> >>> > >> >>> Foreman > >> >>> > >> >>> > > and > >> >>> > > > >> >>> > > > Katello, I’d like to bring some cops that Katello has disabled in > >> >>> > >> >>> its > >> >>> > >> >>> > > > rubocop configuration over to Foreman. These are cops that we’ve > >> >>> > >> >>> decided > >> >>> > >> >>> > > > are a little bit too strict. > >> >>> > > > > >> >>> > > > Currently they are disabled in the rubocop todo file in foreman > >> >>> > >> >>> meaning > >> >>> > >> >>> > > > they are *not* being enforced but they could potentially be if > >> >>> > >> >>> someone > >> >>> > >> >>> > > > removes them from the todo file. > >> >>> > > > >> >>> > > It doesn't look like all of the ones below are disabled in foreman - > >> >>> > >> >>> e.g. > >> >>> > >> >>> > > redundant return, and lambda style. And quite a few of them I like. > >> >>> > > >> >>> > Yea, I guess overlooked some cops. Let me compile a list with the ones > >> >>> > >> >>> that > >> >>> > >> >>> > are enabled in Foreman but disabled in Katello. Then we can discuss if > >> >>> > >> >>> we > >> >>> > >> >>> > want to enable them in Katello or disable them in Foreman. > >> >>> > > >> >>> > > > I’m hoping to get some feedback as to which ones people would like > >> >>> > >> >>> *not* > >> >>> > >> >>> > > to > >> >>> > > > >> >>> > > > be disabled. I’ll collect the feedback and then open a PR based on > >> >>> > >> >>> it. > >> >>> > >> >>> > > For > >> >>> > > > >> >>> > > > reference, here is our rubocop configuration in Katello: > >> >>> > > > > >> >>> > > > > >> >>> > > > > >> >>> > > > https://github.com/Katello/katello/blob/master/.rubocop.yml > >> >>> > > > > >> >>> > > > And here are the cops I’d like to disable: > >> >>> > > > > >> >>> > > > Style/LeadingCommentSpace > >> >>> > > > < > >> >>> > >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Leading > >> >>> >>> > >> >>> > > > Style/IfUnlessModifier > >> >>> > > > < > >> >>> > >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/IfUnles > >> >>> >>> > >> >>> > > > Style/RescueModifier > >> >>> > > > < > >> >>> > >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RescueM > >> >>> >>> > >> >>> > > > Style/AssignmentInCondition > >> >>> > > > < > >> >>> > >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Lint/Assignme > >> >>> >>> > >> >>> > > Why disable this? If you're doing assignment in a conditional, it > >> >>> > >> >>> should > >> >>> > >> >>> > > be wrapped > >> >>> > > in parens to indicate your intention, e.g. if (foo = 'bar'). If you > >> >>> > >> >>> don't > >> >>> > >> >>> > > do that, > >> >>> > > then I think rubocop should complain. This kind of bug could go > >> >>> > >> >>> unnoticed > >> >>> > >> >>> > > if the normal > >> >>> > > case is for the `if` to evaluate as true. > >> >>> > > >> >>> > Originally this cop didn’t allow any assignments in conditions > >> >>> > >> >>> (regardless > >> >>> > >> >>> > of whether you use parentheses or not). However, it looks like now > >> >>> > they > >> >>> > allow it so I’ll enable this cop in Katello and Foreman (unless there > >> >>> > >> >>> are > >> >>> > >> >>> > objections). > >> >>> > > >> >>> > > > Style/WhileUntilModifier > >> >>> > > > < > >> >>> > >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WhileUn > >> >>> >>> > >> >>> > > > Style/AlignParameters > >> >>> > > > < > >> >>> > >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/AlignPa > >> >>> >>> > >> >>> > > > Style/ParenthesesAroundCondition > >> >>> > > > < > >> >>> > >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Parenth > >> >>> >>> > >> >>> > > > Style/DotPosition > >> >>> > > > < > >> >>> > >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/DotPosi > >> >>> >>> > >> >>> > > > Style/Lambda > >> >>> > > > < > >> >>> > >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Lambda > >> >>> > >> >>> > > I thought we agreed to standardize on stabby lambda everywhere > >> >>> > > >> >>> > I don’t remember seeing a discussion for this but I can leave it > >> >>> > >> >>> enabled > >> >>> > >> >>> > unless anyone objects. > >> >>> > >> >>> I guess it wasn't really discussed on the list, the cop was just > >> >>> explicitly > >> >>> enabled and the one-liners all moved to the stabby type: > >> >>> https://github.com/theforeman/foreman/pull/2605 > >> >>> > >> >>> > > > Style/RedundantSelf > >> >>> > > > < > >> >>> > >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Redunda > >> >>> >>> > >> >>> > > > Style/RedundantReturn > >> >>> > > > < > >> >>> > >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Redunda > >> >>> >>> > >> >>> > > I like this cop > >> >>> > > >> >>> > Based on the outcome of this previous discussion ( > >> >>> > https://groups.google.com/forum/#!topic/foreman-dev/77H7AN0wX4g) I’ll > >> >>> > >> >>> leave > >> >>> > >> >>> > this as enabled and enable it in Katello. > >> >>> > > >> >>> > > > Style/SpaceInsideHashLiteralBraces > >> >>> > > > < > >> >>> > >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SpaceIn > >> >>> >>> > >> >>> > > > Style/SingleLineBlockParams > >> >>> > > > < > >> >>> > >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SingleL > >> >>> >>> > >> >>> > > > Style/Next < > >> >>> > >> >>> http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/Next> > >> >>> > >> >>> > > > Style/FormatString > >> >>> > > > < > >> >>> > >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/FormatS > >> >>> >>> > >> >>> > > > Style/GuardClause > >> >>> > > > < > >> >>> > >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/GuardCl > >> >>> >>> > >> >>> > > > Style/StringLiterals > >> >>> > > > < > >> >>> > >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/StringL > >> >>> >>> > >> >>> > > Ditto > >> >>> > > >> >>> > I don’t have a huge preference on this but I will say that it is going > >> >>> > >> >>> to > >> >>> > >> >>> > be a HUGE pain to fix all the places that mix single and double > >> >>> > quotes. > >> >>> > Also, I find it kind of a pain to turn string literals into strings > >> >>> > >> >>> that > >> >>> > >> >>> > can be interpolated. > >> >>> > >> >>> Ok, fair enough, I do agree it's a pain. For some reason I thought > >> >>> using single > >> >>> quotes was faster since it didn't have to look for interpolation but the > >> >>> internets > >> >>> seem to say there's no significant difference. > >> >>> > >> >>> > > > Style/WordArray > >> >>> > > > < > >> >>> > >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WordArr > >> >>> ay > >> >>> > >> >>> > > > Rails/ScopeArgs > >> >>> > > > < > >> >>> > >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Rails/ScopeAr > >> >>> gs > >> >>> > >> >>> > > > Style/EachWithObject > >> >>> > > > < > >> >>> > >> >>> http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/EachWithObject> > >> >>> > >> >>> > > > Style/SymbolProc > >> >>> > > > >> >>> > > > > >> >>> > > > > >> >>> > > > Let me know if there are any questions. Thanks. > >> >>> > > > > >> >>> > > > David > >> >>> > > > > >> >>> > > > -- > >> >>> > > > 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. > >> >>> > > > >> >>> > > -- > >> >>> > > 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. > >> >>> > > >> >>> > -- > >> >>> > 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. > >> >>> > >> >>> -- > >> >>> 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. > >> >> > >> >> -- > >> >> 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. > > > > -- > > 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. > > -- > 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

I think we are sticking to the rubocop config for the most part but the
idea that we MUST enable every rubocop cop without any debate is absurd.
Shopify seems to disable rubocop cops in their projects (e.g.
https://git.io/voNfg). I can’t find where Github is using rubocop but their
Ruby style guide differs quite a bit from the rubocop one (e.g.
https://github.com/styleguide/ruby/hashes vs
https://github.com/bbatsov/ruby-style-guide#symbols-as-keys,
https://github.com/styleguide/ruby/documentation vs
https://github.com/bbatsov/ruby-style-guide#rdoc-conventions). Also,
it looks like Rails (which is probably the biggest Ruby project I know of
in terms of popularity) only uses 6 cops: https://git.io/voNJI.

I think these debates around what cops to enable are beneficial to coming
to a common style that everyone can use and hopefully likes (for the most
part). Again, I think we’ve been enabling a majority of the cops but I
think it’s nonsensical to try to enable every cop. No other project from
what I’ve seen does that.
<https://github.com/Shopify/liquid/blob/master/.rubocop.yml>

David

··· On Wed, Jun 29, 2016 at 6:17 AM, Daniel Lobato Garcia wrote:

I don’t think the purpose of Rubocop or the style guide is really
fulfilled if we start debating about most of them.

  1. The purpose of the style guide is precisely that we don’t argue
    anymore about this (bikeshedding, really) and we just stick to good
    practices (approved by a LOT of companies & individuals, including
    GitHub and Shopify which are 2 of the largest Rails projects out there)

  2. 99% of cops are there for a good reason, including the method size,
    complexity ones, etc… if you feel strongly against any of these and
    you really have solid reasons,
    https://github.com/bbatsov/ruby-style-guide/issues
    is the place.

Having or not having a style-guide is an important decision,
debating every other rule is a triviality & detracts from its purpose.

On 06/28, Ivan Necas wrote:

Thanks David for the poll. It made me realize how terrible the code
would be if all the cops in rubocop were enabled :slight_smile:

– Ivan

On Thu, Jun 23, 2016 at 10:10 AM, Marek Hulán mhulan@redhat.com wrote:

Thank you very much for this effort David.


Marek

On Wednesday 22 of June 2016 10:33:53 David Davis wrote:

Ok, created a poll for the cops that are disabled in Katello but are
outstanding in Foreman. I’ll create one PR for the ones we want to
disable

and then probably follow up with individual PRs for the ones we want
enabled.

http://goo.gl/forms/XQWiGlqrvof8yuWP2

David

On Wed, Jun 22, 2016 at 8:24 AM, David Davis daviddavis@redhat.com > wrote:

I can do a poll. I actually thought about doing it before but since
I am

not planning on actually enabling any of these cops the options
would be

something like “disable” or “do nothing”. Would that make sense to
people?

David

On Wed, Jun 22, 2016 at 4:50 AM, Ivan Necas inecas@redhat.com > wrote:

What about doing a poll for voting for the cops you’re suggesting
to

disable? I’m afraid
the mail thread is not a suitable format for this kind of
questions.

– Ivan

On Tue, Jun 21, 2016 at 6:03 PM, Stephen Benjamin < > stephen@redhat.com> > > >> >> > > >> >> wrote:

----- Original Message -----

From: “David Davis” daviddavis@redhat.com
To: foreman-dev@googlegroups.com
Sent: Tuesday, June 21, 2016 9:58:21 AM
Subject: Re: [foreman-dev] Rubocop cops in Foreman

David

On Tue, Jun 21, 2016 at 9:39 AM, Stephen Benjamin < > stephen@redhat.com> > > >> >>> > > > >> >>> > wrote:

----- Original Message -----

From: “David Davis” daviddavis@redhat.com
To: foreman-dev@googlegroups.com
Sent: Tuesday, June 21, 2016 8:59:54 AM
Subject: [foreman-dev] Rubocop cops in Foreman

In order to have a more consistent rubocop configuration
across

Foreman

and

Katello, I’d like to bring some cops that Katello has
disabled in

its

rubocop configuration over to Foreman. These are cops that
we’ve

decided

are a little bit too strict.

Currently they are disabled in the rubocop todo file in
foreman

meaning

they are not being enforced but they could potentially be
if

someone

removes them from the todo file.

It doesn’t look like all of the ones below are disabled in
foreman -

e.g.

redundant return, and lambda style. And quite a few of them
I like.

Yea, I guess overlooked some cops. Let me compile a list with
the ones

that

are enabled in Foreman but disabled in Katello. Then we can
discuss if

we

want to enable them in Katello or disable them in Foreman.

I’m hoping to get some feedback as to which ones people
would like

not

to

be disabled. I’ll collect the feedback and then open a PR
based on

it.

For

reference, here is our rubocop configuration in Katello:

<
https://github.com/Katello/katello/blob/master/.rubocop.yml>

<
https://github.com/Katello/katello/blob/master/.rubocop.yml>

https://github.com/Katello/katello/blob/master/.rubocop.yml

And here are the cops I’d like to disable:

Style/LeadingCommentSpace
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Leading

Style/IfUnlessModifier
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/IfUnles

Style/RescueModifier
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RescueM

Style/AssignmentInCondition
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Lint/Assignme

Why disable this? If you’re doing assignment in a
conditional, it

should

be wrapped
in parens to indicate your intention, e.g. if (foo = ‘bar’).
If you

don’t

do that,
then I think rubocop should complain. This kind of bug could
go

unnoticed

if the normal
case is for the if to evaluate as true.

Originally this cop didn’t allow any assignments in conditions

(regardless

of whether you use parentheses or not). However, it looks like
now

they
allow it so I’ll enable this cop in Katello and Foreman (unless
there

are

objections).

Style/WhileUntilModifier
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WhileUn

Style/AlignParameters
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/AlignPa

Style/ParenthesesAroundCondition
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Parenth

Style/DotPosition
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/DotPosi

Style/Lambda
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Lambda

I thought we agreed to standardize on stabby lambda everywhere

I don’t remember seeing a discussion for this but I can leave it

enabled

unless anyone objects.

I guess it wasn’t really discussed on the list, the cop was just
explicitly
enabled and the one-liners all moved to the stabby type:
https://github.com/theforeman/foreman/pull/2605

Style/RedundantSelf
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Redunda

Style/RedundantReturn
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Redunda

I like this cop

Based on the outcome of this previous discussion (
https://groups.google.com/forum/#!topic/foreman-dev/77H7AN0wX4g)
I’ll

leave

this as enabled and enable it in Katello.

Style/SpaceInsideHashLiteralBraces
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SpaceIn

Style/SingleLineBlockParams
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SingleL

Style/Next <

http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/Next>

Style/FormatString
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/FormatS

Style/GuardClause
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/GuardCl

Style/StringLiterals
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/StringL

Ditto

I don’t have a huge preference on this but I will say that it
is going

to

be a HUGE pain to fix all the places that mix single and double
quotes.
Also, I find it kind of a pain to turn string literals into
strings

that

can be interpolated.

Ok, fair enough, I do agree it’s a pain. For some reason I
thought

using single
quotes was faster since it didn’t have to look for interpolation
but the

internets
seem to say there’s no significant difference.

Style/WordArray
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WordArr

ay

Rails/ScopeArgs
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Rails/ScopeAr

gs

Style/EachWithObject
<

http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/EachWithObject>

Style/SymbolProc
<
http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/SymbolProc

Let me know if there are any questions. Thanks.

David


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.


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.


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.


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.


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.


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.


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

> From: "Daniel Lobato Garcia" <elobatocs@gmail.com>
> To: foreman-dev@googlegroups.com
> Cc: "David Davis" <daviddavis@redhat.com>
> Sent: Wednesday, June 29, 2016 6:17:45 AM
> Subject: Re: Re: [foreman-dev] Rubocop cops in Foreman
>
> I don't think the purpose of Rubocop or the style guide is really
> fulfilled if we start debating about most of them.
>
> 1. The purpose of the style guide is precisely that we don't argue
> anymore about this (bikeshedding, really) and we just stick to good
> practices (approved by a LOT of companies & individuals, including
> GitHub and Shopify which are 2 of the largest Rails projects out there)
>
> 2. 99% of cops are there for a good reason, including the method size,
> complexity ones, etc… if you feel strongly against any of these and
> you really have solid reasons,
> https://github.com/bbatsov/ruby-style-guide/issues
> is the place.
>
> Having or not having a style-guide is an important decision,
> debating every other rule is a triviality & detracts from its purpose.

I think the effort to standardize on rubocop style across Katello/Foreman
is a good idea, and I think that's what David is trying to do here. Any time
we get the two project orgs to adopt a common set of guidelines brings
us closer together and makes it a lot easier to work across multiple projects. I
don't really think that's trivial.

In fact, I'd like to see maybe a ruby skeleton repo that has things like
the rubocop config for our projects and the ability to sync them so
we can always be the same across the entire codebase. Having one style
would be great.

There are cops that we HAVE to disable (or used to have to) like hash style
because of 1.8.7 compatibility. There are others whose thresholds are
too low for most of us (e.g. line length, method length/complexity, etc) and
we seem to agree on those. There's plenty that I think are just annoying
and exist solely for the sake of existence because someone wanted to do
something cool with Rubocop and opened a PR. This isn't like PEP8 or other
coding styles where they seem pretty stable over time.

Incidentally, Rails enables only a very small number of cops - and ALL the rest
are off:
https://github.com/rails/rails/blob/master/.rubocop.yml

··· ----- Original Message -----

On 06/28, Ivan Necas wrote:

Thanks David for the poll. It made me realize how terrible the code
would be if all the cops in rubocop were enabled :slight_smile:

– Ivan

On Thu, Jun 23, 2016 at 10:10 AM, Marek Hulán mhulan@redhat.com wrote:

Thank you very much for this effort David.


Marek

On Wednesday 22 of June 2016 10:33:53 David Davis wrote:

Ok, created a poll for the cops that are disabled in Katello but are
outstanding in Foreman. I’ll create one PR for the ones we want to
disable
and then probably follow up with individual PRs for the ones we want
enabled.

http://goo.gl/forms/XQWiGlqrvof8yuWP2

David

On Wed, Jun 22, 2016 at 8:24 AM, David Davis daviddavis@redhat.com > > >> wrote:

I can do a poll. I actually thought about doing it before but since I
am
not planning on actually enabling any of these cops the options would
be
something like “disable” or “do nothing”. Would that make sense to
people?

David

On Wed, Jun 22, 2016 at 4:50 AM, Ivan Necas inecas@redhat.com wrote:

What about doing a poll for voting for the cops you’re suggesting to
disable? I’m afraid
the mail thread is not a suitable format for this kind of questions.

– Ivan

On Tue, Jun 21, 2016 at 6:03 PM, Stephen Benjamin > > >> >> stephen@redhat.com > > >> >> > > >> >> wrote:

----- Original Message -----

From: “David Davis” daviddavis@redhat.com
To: foreman-dev@googlegroups.com
Sent: Tuesday, June 21, 2016 9:58:21 AM
Subject: Re: [foreman-dev] Rubocop cops in Foreman

David

On Tue, Jun 21, 2016 at 9:39 AM, Stephen Benjamin > > >> >>> > stephen@redhat.com > > >> >>> > > > >> >>> > wrote:

----- Original Message -----

From: “David Davis” daviddavis@redhat.com
To: foreman-dev@googlegroups.com
Sent: Tuesday, June 21, 2016 8:59:54 AM
Subject: [foreman-dev] Rubocop cops in Foreman

In order to have a more consistent rubocop configuration
across

Foreman

and

Katello, I’d like to bring some cops that Katello has disabled
in

its

rubocop configuration over to Foreman. These are cops that
we’ve

decided

are a little bit too strict.

Currently they are disabled in the rubocop todo file in
foreman

meaning

they are not being enforced but they could potentially be if

someone

removes them from the todo file.

It doesn’t look like all of the ones below are disabled in
foreman -

e.g.

redundant return, and lambda style. And quite a few of them I
like.

Yea, I guess overlooked some cops. Let me compile a list with the
ones

that

are enabled in Foreman but disabled in Katello. Then we can
discuss if

we

want to enable them in Katello or disable them in Foreman.

I’m hoping to get some feedback as to which ones people would
like

not

to

be disabled. I’ll collect the feedback and then open a PR
based on

it.

For

reference, here is our rubocop configuration in Katello:

https://github.com/Katello/katello/blob/master/.rubocop.yml
https://github.com/Katello/katello/blob/master/.rubocop.yml
https://github.com/Katello/katello/blob/master/.rubocop.yml

And here are the cops I’d like to disable:

Style/LeadingCommentSpace
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Leading

Style/IfUnlessModifier
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/IfUnles

Style/RescueModifier
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RescueM

Style/AssignmentInCondition
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Lint/Assignme

Why disable this? If you’re doing assignment in a conditional,
it

should

be wrapped
in parens to indicate your intention, e.g. if (foo = ‘bar’). If
you

don’t

do that,
then I think rubocop should complain. This kind of bug could go

unnoticed

if the normal
case is for the if to evaluate as true.

Originally this cop didn’t allow any assignments in conditions

(regardless

of whether you use parentheses or not). However, it looks like now
they
allow it so I’ll enable this cop in Katello and Foreman (unless
there

are

objections).

Style/WhileUntilModifier
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WhileUn

Style/AlignParameters
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/AlignPa

Style/ParenthesesAroundCondition
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Parenth

Style/DotPosition
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/DotPosi

Style/Lambda
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Lambda

I thought we agreed to standardize on stabby lambda everywhere

I don’t remember seeing a discussion for this but I can leave it

enabled

unless anyone objects.

I guess it wasn’t really discussed on the list, the cop was just
explicitly
enabled and the one-liners all moved to the stabby type:
https://github.com/theforeman/foreman/pull/2605

Style/RedundantSelf
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Redunda

Style/RedundantReturn
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Redunda

I like this cop

Based on the outcome of this previous discussion (
https://groups.google.com/forum/#!topic/foreman-dev/77H7AN0wX4g)
I’ll

leave

this as enabled and enable it in Katello.

Style/SpaceInsideHashLiteralBraces
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SpaceIn

Style/SingleLineBlockParams
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SingleL

Style/Next <

http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/Next>

Style/FormatString
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/FormatS

Style/GuardClause
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/GuardCl

Style/StringLiterals
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/StringL

Ditto

I don’t have a huge preference on this but I will say that it is
going

to

be a HUGE pain to fix all the places that mix single and double
quotes.
Also, I find it kind of a pain to turn string literals into
strings

that

can be interpolated.

Ok, fair enough, I do agree it’s a pain. For some reason I thought
using single
quotes was faster since it didn’t have to look for interpolation but
the
internets
seem to say there’s no significant difference.

Style/WordArray
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WordArr
ay

Rails/ScopeArgs
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Rails/ScopeAr
gs

Style/EachWithObject
<

http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/EachWithObject>

Style/SymbolProc
<http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/SymbolProc

Let me know if there are any questions. Thanks.

David


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.


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.


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.


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.


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.


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.


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.

By the way, thanks to everyone that’s voted so far in the poll. If you
haven’t voted yet, please do so:

http://goo.gl/forms/XQWiGlqrvof8yuWP2

I’ll leave it open until next week before I open a PR against Foreman.

David

··· On Wed, Jun 29, 2016 at 9:32 AM, Stephen Benjamin wrote:

----- Original Message -----

From: “Daniel Lobato Garcia” elobatocs@gmail.com
To: foreman-dev@googlegroups.com
Cc: “David Davis” daviddavis@redhat.com
Sent: Wednesday, June 29, 2016 6:17:45 AM
Subject: Re: Re: [foreman-dev] Rubocop cops in Foreman

I don’t think the purpose of Rubocop or the style guide is really
fulfilled if we start debating about most of them.

  1. The purpose of the style guide is precisely that we don’t argue
    anymore about this (bikeshedding, really) and we just stick to good
    practices (approved by a LOT of companies & individuals, including
    GitHub and Shopify which are 2 of the largest Rails projects out there)

  2. 99% of cops are there for a good reason, including the method size,
    complexity ones, etc… if you feel strongly against any of these and
    you really have solid reasons,
    Issues · rubocop/ruby-style-guide · GitHub
    is the place.

Having or not having a style-guide is an important decision,
debating every other rule is a triviality & detracts from its purpose.

I think the effort to standardize on rubocop style across Katello/Foreman
is a good idea, and I think that’s what David is trying to do here. Any
time
we get the two project orgs to adopt a common set of guidelines brings
us closer together and makes it a lot easier to work across multiple
projects. I
don’t really think that’s trivial.

In fact, I’d like to see maybe a ruby skeleton repo that has things like
the rubocop config for our projects and the ability to sync them so
we can always be the same across the entire codebase. Having one style
would be great.

There are cops that we HAVE to disable (or used to have to) like hash style
because of 1.8.7 compatibility. There are others whose thresholds are
too low for most of us (e.g. line length, method length/complexity, etc)
and
we seem to agree on those. There’s plenty that I think are just annoying
and exist solely for the sake of existence because someone wanted to do
something cool with Rubocop and opened a PR. This isn’t like PEP8 or other
coding styles where they seem pretty stable over time.

Incidentally, Rails enables only a very small number of cops - and ALL the
rest
are off:
https://github.com/rails/rails/blob/master/.rubocop.yml

On 06/28, Ivan Necas wrote:

Thanks David for the poll. It made me realize how terrible the code
would be if all the cops in rubocop were enabled :slight_smile:

– Ivan

On Thu, Jun 23, 2016 at 10:10 AM, Marek Hulán mhulan@redhat.com > wrote:

Thank you very much for this effort David.


Marek

On Wednesday 22 of June 2016 10:33:53 David Davis wrote:

Ok, created a poll for the cops that are disabled in Katello but are
outstanding in Foreman. I’ll create one PR for the ones we want to
disable
and then probably follow up with individual PRs for the ones we want
enabled.

http://goo.gl/forms/XQWiGlqrvof8yuWP2

David

On Wed, Jun 22, 2016 at 8:24 AM, David Davis <daviddavis@redhat.com > > > > > >> wrote:

I can do a poll. I actually thought about doing it before but
since I
am
not planning on actually enabling any of these cops the options
would
be
something like “disable” or “do nothing”. Would that make sense to
people?

David

On Wed, Jun 22, 2016 at 4:50 AM, Ivan Necas inecas@redhat.com > wrote:

What about doing a poll for voting for the cops you’re
suggesting to
disable? I’m afraid
the mail thread is not a suitable format for this kind of
questions.

– Ivan

On Tue, Jun 21, 2016 at 6:03 PM, Stephen Benjamin > > > >> >> stephen@redhat.com > > > >> >> > > > >> >> wrote:

----- Original Message -----

From: “David Davis” daviddavis@redhat.com
To: foreman-dev@googlegroups.com
Sent: Tuesday, June 21, 2016 9:58:21 AM
Subject: Re: [foreman-dev] Rubocop cops in Foreman

David

On Tue, Jun 21, 2016 at 9:39 AM, Stephen Benjamin > > > >> >>> > stephen@redhat.com > > > >> >>> > > > > >> >>> > wrote:

----- Original Message -----

From: “David Davis” daviddavis@redhat.com
To: foreman-dev@googlegroups.com
Sent: Tuesday, June 21, 2016 8:59:54 AM
Subject: [foreman-dev] Rubocop cops in Foreman

In order to have a more consistent rubocop configuration
across

Foreman

and

Katello, I’d like to bring some cops that Katello has
disabled
in

its

rubocop configuration over to Foreman. These are cops that
we’ve

decided

are a little bit too strict.

Currently they are disabled in the rubocop todo file in
foreman

meaning

they are not being enforced but they could potentially
be if

someone

removes them from the todo file.

It doesn’t look like all of the ones below are disabled in
foreman -

e.g.

redundant return, and lambda style. And quite a few of
them I
like.

Yea, I guess overlooked some cops. Let me compile a list with
the
ones

that

are enabled in Foreman but disabled in Katello. Then we can
discuss if

we

want to enable them in Katello or disable them in Foreman.

I’m hoping to get some feedback as to which ones people
would
like

not

to

be disabled. I’ll collect the feedback and then open a PR
based on

it.

For

reference, here is our rubocop configuration in Katello:

<
https://github.com/Katello/katello/blob/master/.rubocop.yml>
<
https://github.com/Katello/katello/blob/master/.rubocop.yml>

https://github.com/Katello/katello/blob/master/.rubocop.yml

And here are the cops I’d like to disable:

Style/LeadingCommentSpace
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Leading

Style/IfUnlessModifier
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/IfUnles

Style/RescueModifier
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RescueM

Style/AssignmentInCondition
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Lint/Assignme

Why disable this? If you’re doing assignment in a
conditional,
it

should

be wrapped
in parens to indicate your intention, e.g. if (foo =
‘bar’). If
you

don’t

do that,
then I think rubocop should complain. This kind of bug
could go

unnoticed

if the normal
case is for the if to evaluate as true.

Originally this cop didn’t allow any assignments in conditions

(regardless

of whether you use parentheses or not). However, it looks
like now
they
allow it so I’ll enable this cop in Katello and Foreman
(unless
there

are

objections).

Style/WhileUntilModifier
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WhileUn

Style/AlignParameters
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/AlignPa

Style/ParenthesesAroundCondition
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Parenth

Style/DotPosition
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/DotPosi

Style/Lambda
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Lambda

I thought we agreed to standardize on stabby lambda
everywhere

I don’t remember seeing a discussion for this but I can leave
it

enabled

unless anyone objects.

I guess it wasn’t really discussed on the list, the cop was just
explicitly
enabled and the one-liners all moved to the stabby type:
https://github.com/theforeman/foreman/pull/2605

Style/RedundantSelf
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Redunda

Style/RedundantReturn
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Redunda

I like this cop

Based on the outcome of this previous discussion (

Redirecting to Google Groups)

I’ll

leave

this as enabled and enable it in Katello.

Style/SpaceInsideHashLiteralBraces
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SpaceIn

Style/SingleLineBlockParams
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SingleL

Style/Next <

Class: RuboCop::Cop::Style::Next — Documentation for rubocop (1.56.4)>

Style/FormatString
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/FormatS

Style/GuardClause
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/GuardCl

Style/StringLiterals
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/StringL

Ditto

I don’t have a huge preference on this but I will say that it
is
going

to

be a HUGE pain to fix all the places that mix single and
double
quotes.
Also, I find it kind of a pain to turn string literals into
strings

that

can be interpolated.

Ok, fair enough, I do agree it’s a pain. For some reason I
thought
using single
quotes was faster since it didn’t have to look for
interpolation but
the
internets
seem to say there’s no significant difference.

Style/WordArray
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/WordArr

ay

Rails/ScopeArgs
<

http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Rails/ScopeAr

gs

Style/EachWithObject
<

Class: RuboCop::Cop::Style::EachWithObject — Documentation for rubocop (1.56.4)>

Style/SymbolProc
<
Class: RuboCop::Cop::Style::SymbolProc — Documentation for rubocop (1.56.4)

Let me know if there are any questions. Thanks.

David


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.


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.


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.


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.


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.


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.


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.