Idea: Use the bbatsov Ruby style guide about guides HOT 21 CLOSED

I'd be willing to try it on some repos. Pretty easy to swap out config with Hound, see how it feels for a while.

from guides.

Alternative title: Joe is tired of making polls

from guides.

👍 👏

• Style guides are mostly useful for consistency. Any styleguide will make it, the bigger the following the better.
• When taste is in the middle with no real technical gains, discussions are subjective (taste!), leading to bikeshed type of discussions (re: how do we align attr_accessible long lists).
• Merging two standards make both act more as standards. Can quote the XKCD comic on that.
• Both projects tackle the same problem and work in similar ways.

from guides.

I haven't read all of this yet, but some of it goes beyond what I would expect hound to cover (and hound likely does not cover it). I think it's important to consider all of what the style guide says rather than just the hound-enforceable bits, right?

For instance:

* Use TODO to note missing features or functionality that should be added at a later date. [link]
* Use FIXME to note broken code that needs to be fixed. [link]
* Use OPTIMIZE to note slow or inefficient code that may cause performance problems. [link]
* Use HACK to note code smells where questionable coding practices were used and should be refactored away. [link]
* Use REVIEW to note anything that should be looked at to confirm it is working as intended. For example: REVIEW: Are we sure this is how the client does X currently? [link]

I'd personally say every one of those should be avoided.

from guides.

I agree with Derek, but I'll re-read the guide and have better feedback later.

from guides.

I'd personally say every one of those should be avoided.

Agreed on master, but I use TODO notes for myself sometimes on code-in-progress.

If there are any parts we particularly disagree with, we can always submit pull requests to their style guide or include revisions in our own.

In case this clarifies: my proposal is to remove most of this file and replace it with a link to the bbatsov guide.

from guides.

@derekprior rubocop/ruby-style-guide#418

from guides.

If there are any parts we particularly disagree with, we can always submit pull requests to their style guide or include revisions in our own.

That does seem reasonable. Worst case we would say, "Use this other style guide with these limited modifications we feel strongly about but could not get merged"

from guides.

Sending PRs upstream is similar to how we use the thoughtbot/dotfiles repository: a big master branch full of conventions and .local prefixed dotfiles for the things we are particular about. I like this flow.

from guides.

I like the above idea of "Use bbatsov's style guide, except these thoughbot specific overrides.". bbatsov's ruby style guide has a lot of rules that most of us are used to and consider good ruby, but it's not explicitly stated in our guides, like:

Indentation of when in a case/when/[else/]end
https://github.com/bbatsov/ruby-style-guide#indent-when-to-case

or

Check indentation of private/protected visibility modifiers.
https://github.com/bbatsov/ruby-style-guide#indent-public-private-protected

or even

Use snake_case for source file names.
https://github.com/bbatsov/ruby-style-guide#snake-case-files

from guides.

From my knowledge of RuboCop and Hound, there are a lot ( of rules enabled by default in RuboCop that are just common sense Ruby to most of us. Since we only want Hound to enforce the guidelines that are explicitly stated in our style guide, it seems very tedious to open PRs for all of the ones I've mentioned in the previous comment.

If you're curious about those rules, here's a Hound PR to explicitly enable only the rules that are in our guides, which would leave out a lot of good stuff (IMO) before we add them to our guides.

from guides.

There's a lot of stuff in there as well that we don't like, and I'm actually not a fan of the ".local" convention (anywhere, but especially) for the style guide because it means there's not a canonical place to go.

Examples:

• favor trailing conditionals over one line within a conditional https://github.com/bbatsov/ruby-style-guide#if-as-a-modifier
• unless ever https://github.com/bbatsov/ruby-style-guide#unless-for-negatives
• use assignment results in conditionals, but wrap them in parenthesis https://github.com/bbatsov/ruby-style-guide#safe-assignment-in-condition
• don't check nil (and I'd assume a corollary to blank, present), rather check implicit checks https://github.com/bbatsov/ruby-style-guide#no-non-nil-checks
• return ever https://github.com/bbatsov/ruby-style-guide#no-nested-conditionals

Many of these are opinion decisions, which we're unlikely to be able to convince the bbatsov guide to agree with and we won't enjoy reading or writing.

from guides.

I'm actually not a fan of the ".local" convention (anywhere, but especially) for the style guide because it means there's not a canonical place to go.

I agree with this a lot. I would prefer to use a few rules I don't agree with than to have to multiple places to look. There is no way you would know which rules from the original repo were overridden, if you looked only there and I think having to look in two different places is too much overhead.

Personally, I don't necessarily agree with a lot of the opinions in those guides (ternaries, trailing conditionals), but I don't mind enough to fight most of it and I'd be happy to just follow a community guide.

from guides.

I'd be willing to try it on some repos.

This feels like the right next step. If somebody is interested in trying this out on their current project, that would likely help us get a feel for what it's like to use the bbatsov guide.

from guides.

I created a Research card for anybody interested in trying out the bbatsov guide: https://trello.com/c/XlbrTx9I/470-use-the-bbatsov-ruby-style-guide

from guides.

I just did a pass-through of this style guide, and I think we'd agree, except for these cases (some opinions follow):

• When assigning the result of a conditional expression to a variable, preserve the usual alignment of its branches -- don't align things more than 2 spaces beyond the last indentation level
• Align the parameters of a method call if they span more than one line -- don't align things more than 2 spaces beyond the last indentation level
• Stuff about ternary operators:
  • https://github.com/bbatsov/ruby-style-guide#ternary-operator
  • https://github.com/bbatsov/ruby-style-guide#no-nested-ternary
  • https://github.com/bbatsov/ruby-style-guide#no-semicolon-ifs
• Favor modifier if / unless usage when you have a single-line body -- don't use modifiers
• Favor modifier while / until usage when you have a single-line body -- don't use modifiers
• Comment annotations, as previously mentioned (that said, I don't think this is terrible; I could see it being used with rake notes. But these should probably be added as cards in Trello/Switchboard/whatever instead)
• Prefer modules to classes with only class methods -- try not to use class methods to begin with
• Prefer alias when aliasing methods in lexical class scope -- not sure about this one, but I think you can just get away with using alias_method all the time
• Prefer %w to the literal array syntax and Prefer %i to the literal array syntax -- I'd actually advocate not using these, since I like to see explicit strings or symbols colored like that in my editor (with %w and %i you lose the coloring).
• Avoid comma after the last item of an Array or Hash literal -- didn't we have a PR about this?
• When accessing the first or last element from an array, prefer first or last over [0] or [-1] -- this seems like an arbitrary rule. I'd advise against since it encourages using second, third, etc. in Rails-land.

from guides.

I don't quite understand what the .local convention would be. Couldn't we still maintain our Ruby guide page, that states something like:

Use bbatsov's guide (link), except these:

* Avoid trailing conditionals.
* Don't use ternary expressions.
...

from guides.

@jferris what's the current thinking on this, given developments in dev style and/or Hound in the last 9 months?

from guides.

@mjankowski I haven't been on a new project where I could try this out, and it doesn't seem like anybody else is super interested. I'll keep an eye on projects as they start up this quarter to see if I can finally get some traction here.

from guides.

The Trello card was archived, so let's close this issue. Yeah?

from guides.

Yeah, I think that makes sense. If anybody decides to try this out, feel free to re-open for discussion.

from guides.