----- 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.
-
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)
-
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:
rails/.rubocop.yml at main · rails/rails · GitHub
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 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:
<
katello/.rubocop.yml at master · Katello/katello · GitHub>
<
katello/.rubocop.yml at master · Katello/katello · GitHub>
katello/.rubocop.yml at master · Katello/katello · GitHub
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:
Refs #3809 - Stabby lambda syntax for oneliners by dLobatog · Pull Request #2605 · theforeman/foreman · GitHub
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.63.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.63.4)>
Style/SymbolProc
<
Class: RuboCop::Cop::Style::SymbolProc — Documentation for rubocop (1.63.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.
