David On Tue, Jun 21, 2016 at 9:39 AM, Stephen Benjamin <step...@redhat.com> wrote:
> > > ----- Original Message ----- > > From: "David Davis" <davidda...@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. > > > 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+unsubscr...@googlegroups.com. > > For more options, visit https://groups.google.com/d/optout. > > > > -- > You received this message because you are subscribed to the Google Groups > "foreman-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to foreman-dev+unsubscr...@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "foreman-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to foreman-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
