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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> >> >> >> >> wrote: >> >>> ----- 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/Leading >> >>> CommentSpace>>> >> >>> > > > Style/IfUnlessModifier >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/IfUnles >> >>> sModifier>>> >> >>> > > > Style/RescueModifier >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/RescueM >> >>> odifier>>> >> >>> > > > Style/AssignmentInCondition >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Lint/Assignme >> >>> ntInCondition>>> >> >>> > > 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 >> >>> tilModifier>>> >> >>> > > > Style/AlignParameters >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/AlignPa >> >>> rameters>>> >> >>> > > > Style/ParenthesesAroundCondition >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Parenth >> >>> esesAroundCondition>>> >> >>> > > > Style/DotPosition >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/DotPosi >> >>> tion>>> >> >>> > > > 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 >> >>> ntSelf>>> >> >>> > > > Style/RedundantReturn >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/Redunda >> >>> ntReturn>>> >> >>> > > 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 >> >>> sideHashLiteralBraces>>> >> >>> > > > Style/SingleLineBlockParams >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/SingleL >> >>> ineBlockParams>>> >> >>> > > > 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 >> >>> tring>>> >> >>> > > > Style/GuardClause >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/GuardCl >> >>> ause>>> >> >>> > > > Style/StringLiterals >> >>> > > > < >> >>> >> >>> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/StringL >> >>> iterals>>> >> >>> > > 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 [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. >> >> >> >> -- >> >> 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.
