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 <davidda...@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 <ine...@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 <step...@redhat.com> >> wrote: >> >>> >>> >>> ----- Original Message ----- >>> > From: "David Davis" <davidda...@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 <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. >>> >>> >>> 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+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. >>> > >>> >>> -- >>> 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.
