----- Original Message ----- > From: "David Davis" <[email protected]> > To: [email protected] > 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 <[email protected]> > wrote: > > > > > > > ----- Original Message ----- > > > From: "David Davis" <[email protected]> > > > To: [email protected] > > > 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 [email protected]. > > > 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 [email protected]. > > 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 [email protected]. > 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 [email protected]. For more options, visit https://groups.google.com/d/optout.
