I think we are sticking to the rubocop config for the most part but the idea that we MUST enable every rubocop cop without any debate is absurd. Shopify seems to disable rubocop cops in their projects (e.g. https://git.io/voNfg). I can’t find where Github is using rubocop but their Ruby style guide differs quite a bit from the rubocop one (e.g. https://github.com/styleguide/ruby/hashes vs https://github.com/bbatsov/ruby-style-guide#symbols-as-keys, https://github.com/styleguide/ruby/documentation vs https://github.com/bbatsov/ruby-style-guide#rdoc-conventions). Also, it looks like Rails (which is probably the biggest Ruby project I know of in terms of popularity) only uses 6 cops: https://git.io/voNJI.
I think these debates around what cops to enable are beneficial to coming to a common style that everyone can use and hopefully likes (for the most part). Again, I think we’ve been enabling a majority of the cops but I think it’s nonsensical to try to enable every cop. No other project from what I’ve seen does that. <https://github.com/Shopify/liquid/blob/master/.rubocop.yml> David On Wed, Jun 29, 2016 at 6:17 AM, Daniel Lobato Garcia <elobat...@gmail.com> wrote: > I don't think the purpose of Rubocop or the style guide is really > fulfilled if we start debating about most of them. > > 1. 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) > > 2. 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, > https://github.com/bbatsov/ruby-style-guide/issues > 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. > > 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 <mhu...@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 <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/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 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. > > > > -- > > 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. > > -- > Daniel Lobato Garcia > > @dLobatog > blog.daniellobato.me > daniellobato.me > > GPG: http://keys.gnupg.net/pks/lookup?op=get&search=0x7A92D6DD38D6DE30 > Keybase: https://keybase.io/elobato > -- 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.
