• Edit the VERSION constant or whatever, making the version change
• Run bundle install in the gem itself so the Gemfile.lock updates
• Run the test suite to make sure things are good
• Edit the NEWS/Changelog/etc if relevant
• Commit your changes with an announcement of the version bump ("Bump to version 2.1.0")
• Run rake release, which pushes a commit announcing the version bump, tags the release, pushes the gem to Rubygems.org, and pushes the new tagged release to GitHub

This is the opposite of what I see in the ruby sample right now.

I like this:

class Thingy
  def self.hello
    puts 'Hello'
  end

  def hi
    puts 'Hi'
  end
end

Do people prefer writing multi-line expect blocks using delimiters or do end.

expect {
  some_long_method_name_that_does_not_fit_on_one_line
}.to change { something }


expect do
  some_long_method_name_that_does_not_fit_on_one_line
end.to change { something }

I generally prefer the first example over the latter and I wondered if we should add a new rule to our style guide.

Use {...} for multi-line expect or it blocks.

Here is a link to a discussion me and @gylaz had about this over at hound.

houndci/hound#270

https://github.com/bbatsov/ruby-style-guide

We're not crazy far away from this guide. Using it would have the following advantages:

• Our code is more likely to look like community code.
• We no longer have to argue over styles which are largely only important for consistency.
• We don't have to configure Hound/Rubocop, as it knows how to use the bbatsov guide out of the box.
• We'd be contributing to the community effort rather than giving into NIH syndrome.

If there any parts of that guide people truly hate, we could always have revisions in our own guide and use theirs as a base.

Please discuss. If it seems like there is support for this, I will open a poll.

Isn't better to use tabs to indent the code in order to let the user/reader decide how many space she wants to use to indent? If you choose 2 spaces and I prefer 4 I will need to reformat the whole file. Instead, if you choose to use tabs for indentation and specify 2 space tabs in your editor, when I get your file it will adapt automatically to my 4 space tabs.

Isn't better to decouple content from presentation?

Readme for url "Exceptions should be exceptional" does not resolve.

https://github.com/thoughtbot/guides/blame/master/best-practices/README.md#L17

Can we revisit #133 with respect to thoughtbot/shoulda-matchers#567 ?

I just ran into the Non-NULL database column problem, where validates_uniqueness_of didn't know how to create a valid record.

Here's what the guides suggested my specs look like:

  describe "Validations" do
    it "validates uniqueness of email" do
      create(:registration)

      expect(Registration.new).to validate_uniqueness_of(:email)
    end

    it "validates uniqueness of mobile phone number" do
      create(:registration)

      expect(Registration.new).to validate_uniqueness_of(:mobile_phone_number)
    end

    it { is_expected.to validate_presence_of(:email) }
    it { is_expected.to validate_presence_of(:mobile_phone_number) }
  end

And here's what it looks like with an explicit subject:

  describe "Validations" do
    subject { build(:registration) }

    it { is_expected.to validate_presence_of(:email) }
    it { is_expected.to validate_presence_of(:mobile_phone_number) }
    it { is_expected.to validate_uniqueness_of(:email) }
    it { is_expected.to validate_uniqueness_of(:mobile_phone_number) }
  end

I would suggest that the second option is more readable and more manageable, with less duplication. Thoughts on allowing an explicit subject for use with shoulda-matchers?

Hi,

I have an open question, I would like to clearly differentiate public and private interface in a class.
Usually, the recommended best practice is to define public methods on top, and private methods on bottom like this:

class MyClass
  def a_public_method
  end

  private
  def a_private_method
  end
end

The problem is as the class becomes bigger, the split between public and private becomes less and less visible. (And sometimes you have big, enormous classes, and no time to refactor)

So some of my first guess would be to declare private in front of each private methods, like the languages of the old days:

class MyClass
  def a_public_method
  end

  private def a_private_method
  end

  private def another_private_method
  end
end

Or, to avoid the private keyword, an define a naming convention, like putting an underscore in front of each private methods:

class MyClass
  def a_public_method
  end

  def _a_private_method
  end
end

In a result, none of these two methods have really satisfied me, the first one is too verbose, and the second one leads to a lower readability because of the underscores. So my last guess was to use the public keyword in front of every public method, and still split the methods definition between public and private, like this:

class MyClass
  public def a_public_method
  end

  private 
  def a_private_method
  end
end

I think this would be less verbose, as there often is less public methods than private ones, and it reveals clearly the public interface to anyone who read the code.

So, what do you think of that ? How do you do, and what should be the best practice for you?
Thanks.

PS: Is it the good repo for this?

https://gist.github.com/harlow/3960799

Which one is better?

• script-name.coffee
• script_name.coffee

What is the reason for this? How do you DRY up common setup code?

Is there any chance of getting hound to use the latest Rubocop? it has different cop names than the older version cops. (added Style/ and Lint/ namespaces)

I recently tried to organize my stylesheets workflow which has always been somehow messy. I stumbled upon some posts which promoted the idea of using SCSS for non-CSS-outputting code (@mixins, %placeholders, $variables) while using Sass (indent syntax) for everything that ouputs CSS.

So the logically resulting project structure I used has been this:

stylesheets/
  modules/
    header/
    main-nav/
    footer/
      _mixins.scss
      _variables.scss
      _footer.sass
  _variables.scss
  _placeholders.scss
  _mixins.scss
  application.scss # only @import statements

This is nice since I can write mixins, functions and placeholders more carefully and in a more structured way. For example, I can use one line blocks like

@mixin box-sizing($type) {
  -webkit-box-sizing { box-sizing: $type }
  -mox-box-sizing { box-sizing: $type }
  box-sizing { box-sizing: $type }
}

which is way clearer then using three lines per vendor prefix.

Writing actual CSS-outputting code in Sass has its advantages too.
For example I can @include mixins using using just +. Without having to write {}; everywhere, I feel more proficent and the resulting code looks cleaner to me.

Also, this helps with separation of logic (.scss) and presentation (.sass).

What do you think? I'm curious about this technique.

One of the neatest features of Ruby is that it so often comes out looking like natural English. But I think that following this Ruby style guide would inhibit this feature. I think that if the code reads like natural English and that English phrase or sentence actually describes what the code is doing, then this should take precedence over preferring one method over another or avoiding conditional modifiers.

for example this code reads naturally but wouldn't so much if it didn't use the conditional modifier:
redirect_to root_path unless current_user.can_create_report?

Thanks for sharing the protocol guide, it's quite helpful.

I have 2 questions if you could elaborate on so I'm clear:

1. You specify an app restart post migration. We have issues sometimes I call "stuck dynos" where sometimes not every process seems to be aware of new columns. Is this to alleviate that, or is there another reason?
2. When you introspect to make sure "everything is ok" what do you look for?

Rubygems allows you to yank specific versions of a gem. A yanked version isn’t available through bundle install or gem install.

Yanking gems is confusing and can break production deploys.

Yanking a gem should only be used as a last resort when installing that particular version could cause serious damage to the user.

Yanked versions cannot be overwritten, and are usually inappropriate for withdrawing accidental releases.

Note that yanking a gem because of leaked information like passwords is insufficient, because you can still download yanked gems directly from the Rubygems web site.

When applicable, it would be useful to have the style guides be accompanied by a linter recommendation and a config file for that linter. This would make it easier to enforce these recommendations programmatically as well as ease adoption for those interested.

Hound may soon take the place of these static code analysis tools but until additional languages are supported I think this is a good alternative for those that cannot use Hound for whatever reason.

I started translating the SCSS guidelines into an SCSS-Lint config but was unsure where to add the new file. The style/README.md file is already quite long. Mirroring the structure of "Protocol" makes sense to me. Each section could be broken out into a directory with a README, lint config and code sample:

style/
├── README.md
└── sass
    ├── README.md
    ├── sample.scss
    ├── .scss-lint.yml

Though this may be a moot point if the repository is being restructured at all for the move to GitHub Pages in #142.

Thoughts?

Hey guys,

Do you have any guides or thoughts around gem creation?

For instance, if we look at your fake_braintree gem it's easy to see a bunch of best practices in use.

• You have a contributing doc
• A great README that is very well structured
• A license file, etc.

The gem structure itself (lib folder) is handled by bundler and the like, but even so, it'd still be awesome to have a suspenders-like utility to generate an open source gem repository fully set up in a way that you guys at Thoughtbot recommend, including all the docs and licenses you recommend etc.

I think a lot of intermediate rails developers (like myself) have a lot of ideas for gems, but the job of extracting things and getting set up to create an open source repo the correct way and to abstract the code is just too ambiguous. And this problem is magnified when the idea to create a gem usually strikes, which is usually in the middle of some large or complicated project, thus we just push off creating the gem to later. Anyway, what I'm getting at is I think such a utility could help out a little bit with that, if nothing else. :)

PS - If this already exists my apologies, but I couldn't find it.

Under Best Practices > General, "Exceptions should be exceptional" links to http://www.readability.com/~/yichhgvu.

This link redirects to http://pragmatictips.com/34, which is dead (and it appears has been for a while).

Here's a cached version: http://webcache.googleusercontent.com/search?q=cache:b9TMRPZjC_EJ:pragmatictips.com/34+&cd=2&hl=en&ct=clnk&gl=us.

I've mirrored the content here and I can submit a PR replacing the link if you're interested.

Good podcast the other day from Joe and Mike. One topic was method order.

Sounds like we're coming to consensus as a team to not order methods alphabetically. Instead, use the Uncle Bob method of highest level of abstraction up top, private methods at the bottom.

Sounds good to me.

I'm pretty warped but I appreciate the neatness and readability of alphabetizing in some other areas and would like feedback / support from others on alphabetizing the following other areas (not methods):

• config/routes.rb
• spec/factories.rb
• attr_accessible / strong_parameters lists
• validations and their Shoulda specs
• associations and their Shoulda specs
• have_db_column specs
• CSS attributes

Who's with me? Follow me to freedom.

There are currently several ways in Ruby to break up long method chains.

First:

stub_request(:put, url).
  with(
    headers: {
      "Authorization" => "some_token",
      "Accept" => "some_media_type",
    }
  )

Second:

stub_request(:put, url).with(
  headers: {
    "Authorization" => "some_token",
    "Accept" => "some_media_type",
  }
)

I believe we prefer the first -- it makes it easier to see that the arguments are for with. We also have a guideline that backs this:

If you break up a chain of method invocations, keep each method invocation on its own line. Place the . at the end of each line, except the last

However, what if we need to chain something else to these calls:

stub_request(:put, url).
  with(
    headers: {
      "Authorization" => "some_token",
      "Accept" => "some_media_type",
    }
  ).
  to_return(
    body: "some body text",
    status: 200
  )

That looks awkward, but it follows the style of the "First" scenario. Naturally we'd do this:

...
      "Accept" => "some_media_type",
    }
  ).to_return(
    body: "some body text",
    status: 200
  )

but then that break consistency with how with is broken to its own line.

We could impose a rule like, always bring the closing paren and the dot, when chaining. Resulting in:

stub_request(
  :put,
  url
).with(
  headers: {
  "Authorization" => "some_token",
  "Accept" => "some_media_type",
  }
).to_return(
  body: "some body text",
  status: 200
)

My question is: is it worth standardizing one way for things like this? Or is "use your best judgement" a good enough solution here?

Why is inject preferred? Isn't reduce better, following the map/reduce model?

Currently in protocol:

bundle --binstubs

Improve to:

bundle --binstubs=bin/stubs

This question refers to the SASS style guide.

What are the feelings on using SASS nested properties? I have something like:

text-align: ...;
text-decoration: ...;
text-transform: ...;

And I'm wondering if its better to use the following in this case:

text: {
  align: ...;
  decoration: ...;
  transform: ...;
}

And whether you would do the same for two declarations as well?

font-size: ...;
font-weight: ...;

to

font: {
  size: ...;
  weight: ...;
}

Use Neat for a grid framework.

Is this really necessary? Don't get me wrong, I love grids, but only for use in graphic design and architecture. Why some people see the need to transfer them programatically onto their HTML and CSS is beyond me. If you absolutely need a grid, use something like http://hashgrid.com/.

Ref. http://www.slideshare.net/huer1278ft/grids-are-good-right
http://vignelli.com/canon.pdf

Sass

Some people would argue that if ones CSS is so complex that it requires Sass, it's probably not that good in the first place.

Sure it allows you to DRY some things up, but in return you have to add yet another complex layer of abstraction to your stack, plus it makes it hard to work with designers who prefer the minimalist way of prototyping layouts in pure HTML and CSS.

Perfection is achieved, not when there's nothing left to add, but when there's nothing left to take away.

Ref. http://motherfuckingwebsite.com/

CoffeeScript

See the above, plus:

1. Could we add some jQuery styles here as well for the overwhelming majority of us who prefer not to use CoffeeScript?
2. How do you guys feel about using dollar signs in variables? I hear a lot of people prefer dropping them.

Ref. http://procbits.com/2012/05/18/why-do-all-the-great-node-js-developers-hate-coffeescript

Use Normalize as a browser reset.

Is that really necessary? Won't say * { margin: 0; padding: 0; box-sizing: border-box; } suffice?

Per our conversation, @mike-burns, it'd be great to get shell scripting guidelines and examples in here.

"Avoid its, let, let!, specify, and before in RSpec."

How to "setup" common variables for RSpec tests then?

Might be clearer and more persuasive to see before/after examples of code for style examples only.

Some of the discussions in Guides PRs lately leads me to believe that a PR to the README of the guides repo clarifying what the entire purpose of the guides is, might be beneficial in steering these conversations towards being more focused and productive.

There's very little in the way of high level guidance around how to think about these things, and I suspect that encountering all of this as a new employee or random internet stranger could be confusing right now.

Are these rules to always enforce on every project?

Are these a starting point which we expect many projects to override?

Are these just a snapshot in time and we expect some churn in here as preferences evolve?

Are these supposed to be limited only to things we ALL agree on? That just a consensus agree on?

Could you give some background behind your

Use one ActionMailer for the app. Name it Mailer.

Under Reviewing Code
Understand why the code is necessary (bug, user experience, refactoring).

I think it'd be better write bugfix - bugs in general aren't necessary (well - except maybe for some strange edge cases XD )

https://github.com/thoughtbot/guides/tree/master/best-practices#shell

Don't parse the output of ls. See here for details and alternatives.

http://wooledge.org/ doesn't look like its available at the moment.

https://github.com/thoughtbot/guides/blob/master/protocol/git/README.md

git fetch origin
git rebase -i origin/master

Sometimes, small teams will work on a branch, but typically on different files within that branch.

Possibly the advice on rebasing should explain the pitfalls of doing the above rebase without coordinating with other branch users.

From #140: move this to gh-pages visually, similar to the Playbook.

With RSpec mocks, the following:

receiver.stub(:message).and_return(:return_value)

is equivalent to:

receiver.stub(message: :return_value)

I prefer the second. Any objections/ideas?

In /best-practices#rails, the given best practice is

If there are default values, set them in migrations.

Why is it better to set default values in migrations?

Shamelessly ripped from LevelUp. Passing off to Mark for his edits.

• In general, all compiler warnings in Objective-C should be treated as errors and corrected. Warnings of form

  'SomeClass' may not respond to '-someSelector'

  always indicate a scope issue, and even though these selectors may be resolved at runtime in certain cases, you should always resolve these warnings by importing the appropriate files and, in rare cases, by typecasting your objects explicitly when selector naming conflicts arise.

• Use @class to forward declare classes in class interfaces and #import to load the actual headers in the corresponding class implementation. For example, do the following in ClassA.h:

  @class ClassB;
  @interface ClassA {
    ClassB * classBInstance;
  }
  

  and the following in ClassA.m:

  #import "ClassB.h"
  

• When defining an inheritance or protocol conformance relationship, be as specific as possible with #import statements in the class interface:

  #import "Three20/TTView.h" // Recommended; specific
  #import "Three20/Three20.h" // Not recommended; unnecessary
  @interface MyView : TTView { ... }

• Strictly type objects whenever possible. Loosely type objects only when necessary. id is a special type in Objective-C, and the compiler will not generate warnings for any messages passed to an object of type id.

• Where available, use class initializers rather than autoreleased instance initializers:

  NSArray* array1 = [NSArray array];                       // Already autoreleased
  NSArray* array2 = [[[NSArray alloc] init] autorelease];  // init then autorelease; unnecessary
  

• For transient instances, use autorelease rather than init followed by release:

  NSArray* array1 = [NSArray array];          // autoreleased (array1 retain count: effectively 0)
  instance1.collection = array1;              // retained (array1 retain count: 1)
  NSArray* array2 = [[NSArray alloc] init];   // retained (array2 retain count: 1)
  instance2.collection = array2;              // retained (array2 retain count: 2)
  [array2 release];                           // released (array2 retain count: 1)
  

• Always use YES and NO as BOOL values:

  BOOL boolValueA = YES;          // Recommended
  BOOL boolValueB = false;        // Inadvisable
  

• Do not explicitly compare BOOL values to YES or NO in boolean logic expressions:

  BOOL boolValue = NO;
  ...
  if (boolValue) { ... }            // Acceptable for BOOL types
  if (boolValue == YES) { ... }     // Unnecessary
  

• Explicitly compare pointer types to nil in boolean logic expressions:

  SomeClass* someInstanceA = nil;
  SomeClass* someInstanceB = nil;
  ...
  if (someInstanceA != nil) { ... }  // Recommended
  if (someInstanceB) { ... }         // Ambiguous; BOOL or pointer type?
  

• Avoid using the app delegate -- the class that conforms to the UIApplicationDelegate protocol -- as an all-purpose utility singleton. Only instance variables and methods that relate directly to the flow of the application should be contained within the app delegate.

Style

• Always follow the Coding Guidelines for Cocoa as well as the Google Objective-C Style Guide.

• Join pointers to their type followed by a single space: NSString* someString;

• Sort @class statements alphabetically:

  @class CLLocationManager;
  @class TTURLRequestModel;
  @class ZClass;
  

• Group #import statements by "framework", sorting them alphabetically within each group. Actual Apple frameworks should be first:

  #import <Foundation/Foundation.h>  // Apple
  #import <objc/message.h>           // Apple
  #import <objc/runtime.h>           // Apple
  #import <UIKit/UIKit.h>            // Apple
  #import "Three20/Three20.h"        // Three20
  #import "AClass.h"                 // Application
  #import "ZClass.h"                 // Application
  

• Group method prototypes by static and instance methods, sorting them alphabetically by selector within each group:

  + (void)staticMethodWithClassA:(A*)a classB:(B*)b;
  + (void)staticMethodWithClassA:(A*)a classC:(C*)c;
  - (NSString*)instanceMethodWithClassA:(A*)a classB:(B*)b;
  - (NSString*)instanceMethodWithClassA:(A*)a classC:(C*)c;
  

• Only list method prototypes in a class interface if it is specific to that class. Do not list inherited method, conformed protocol (formal or informal) method prototypes, synthesized @property methods, etc.:

  - (id)initWithString:(NSString*)string label:(UILabel*)label;    // Appropriate; class-specific
  - (id)init;                                                      // Not appropriate; inherited from NSObject
  - (void)setValue:(id)value forUndefinedKey:(NSString*)key;       // Not appropriate; conforms to NSObject (NSKeyValueCoding)
  

• Use #pragma mark to group method implementations in the following order, sorting methods alphabetically by selector within each group:

  1. Inline class category methods from the inline class category interface.

  2. Class extension methods from the class extension interface. Add "(Private)" to the #pragma mark label for these methods and "(Public)" to the #pragma mark label for class methods.

  3. Class methods from the class interface.

  4. Subclass methods from the subclass interface and in the order in which the class inherits from subclasses in the inheritance hierarchy.

  5. Formal protocol methods from the protocol documentation and in the order in which the protocols are adopted in the class interface.

  6. Informal protocol methods from the informal protocol documentation and in alphabetical order by the categorized class name first and by the informal protocol name second.

     For example, assuming ClassC inherits from ClassB -- which inherits from ClassA, which inherits from NSObject -- conforms to the ProtocolD and ProtocolE formal protocols, conforms to the NSObject (NSKeyValueCoding) and NSObject (UINibLoadingAdditions) informal protocol, and has both a named class category and a class extension, ClassC.m would be look like the following:

     #pragma mark -
     #pragma mark ClassC (InlineClassCategory) Methods
     ...
     #pragma mark -
     #pragma mark ClassC Methods (Private)
     ...
     #pragma mark -
     #pragma mark ClassC Methods (Public)
     ...
     #pragma mark -
     #pragma mark ClassB Methods
     ...
     #pragma mark -
     #pragma mark ClassA Methods
     ...
     #pragma mark-
     #pragma mark NSObject Methods
     ...
     #pragma mark -
     #pragma mark ProtocolD Methods
     ...
     #pragma mark -
     #pragma mark ProtocolE Methods
     ...
     #pragma mark -
     #pragma mark NSObject (NSKeyValueCoding) Methods
     ...
     #pragma mark -
     #pragma mark NSObject (UINibLoadingAdditions) Methods
     ...
     

• Although Apple suggests that the word "Additions" be appended to a framework class name when creating a category on that class, this naming convention is very likely to result in naming conflicts when using multiple third-party frameworks/libraries. Instead, name framework class categories as follows: NSString+XYZAdditions.h, where XYZ is the chosen private API prefix or NSString+DescriptiveNameAdditions.h, where DescriptiveName somehow describes what the addition does.

• In the dealloc method, release all non-weakly-associated instance variables and set all instance variables to nil:

  [instanceVariableA release];  // Strong association
  [instanceVariableB release];  // Strong association
  instanceVariableA = nil;      // Strong association
  instanceVariableB = nil;      // Strong association
  instanceVariableC = nil;      // Weak association
  

I generally don't use instance variables in tests - I believe this was part of Josh's TDD Workshop curricula but I'm not sure.

Is there a stance on this? If so, should go into the testing section.

I'm curious if this format is preferred:

items.map(&:some_attribute)

Over the listed:

items.map { |item| item.some_attribute }

Same thing for other common blocks, like select.

Happy to submit a pull request, just curious if this was an over-site, or a conscious part of the thoughtbot style guide (which I am in love with).

Could you please elaborate on this? What If I need to have some additional collections? Use helper_methods? Write Model.all in the view?

So, given this line:

attr_accessor :email, :name, :github_username, :password, :stripe_customer_id,
  :stripe_subscription_id, :stripe_token, :organization, :address1, :address2,
  :city, :state, :zip_code, :country

We've been doing this for so long, since we have these rules:

• Don't vertically align tokens on consecutive lines.
• Indent continued lines two spaces.

However, we have this rule:

• If you break up an argument list, keep the arguments on their own lines and closing parenthesis on its own line.

... which after discussion with @gylaz it seems like we have to do this because of that rule:

attr_accessor(
  :email,
  :name,
  :github_username,
  :password,
  :stripe_customer_id,
  :stripe_subscription_id,
  :stripe_token,
  :organization,
  :address1,
  :address2,
  :city,
  :state,
  :zip_code,
  :country
)

I feel like the intention of that rule wasn't really intended for us to do that, as I remembered a lot of project that we have shitton of stuff in the same line and indent two spaces on the next line is just fine.

Do you think we should amend the rule so 1st code block form is acceptable? Or, was it that we stop doing it for so long and I just realized about it?

Are we talking about keeping both the open and the closed parentheses on their own new lines? What about the curly braces?

If we're only talking about the closing paren/curly, then I would suggest specifying that.

Our open source protocol currently says to release using rake release:

https://github.com/thoughtbot/guides/tree/master/protocol/open-source

If I'm understanding things correctly, this results in a git tag, but not a GitHub release, being created like this:

https://github.com/thoughtbot/suspenders/releases/tag/v1.15.0

I think there needs to be a step after rake release where we write a nice message at https://github.com/thoughtbot/suspenders/releases/new?tag=v1.15.0 so it looks more like the nice one @mjankowski wrote at https://github.com/thoughtbot/suspenders/releases/tag/v1.14.0. Is that correct?

Instead of the app/views/shared convention that is fairly common in the community, if we use application as our standard, we get a couple of nice things that the framework does for us:

http://asciicasts.com/episodes/269-template-inheritance

Code should only return at the end of the function
Not only does it make for cleaner code, but also having multiple exit point can get out of hand and the compiler makes optimisations with the return at an end of a function.

• The single exit point gives an excellent place to assert your post-conditions.
• Being able to put a debugger breakpoint on the one return at the end of the function is often useful.
• Fewer returns means less complexity. Linear code is generally simpler to understand.
• If trying to simplify a function to a single return causes complexity, then that's incentive to refactor to smaller, more
  general, easier-to-understand functions.
• A single return reduces the number of places you have to clean up.

http://stackoverflow.com/questions/36707/should-a-function-have-only-one-return-statement

There's some good stuff in the move-out file in the move-out branch.

The top of the file says:

This is a list of rules that were taken out of the style guide because they had wider implications that cosmetic conventions. Please move them to a new home if you find one.

If cosmetic is the rule for the style-guide, there are other items in the current style guide that should be moved out:

• Have Heroku remotes named "staging" and "production."
• Use a Procfile to configure your services.
• Use the Cedar stack.
• Avoid including files in source control that are specific to your development machine or process.
• Delete local and remote feature branches after merging.
• Perform work in a feature branch.
• Prefix feature branch names with your initials.
• Rebase frequently to incorporate upstream changes.
• Use a pull request for code reviews.
• Write a good commit message.
• Define the project's Ruby version in the Gemfile.
• Use the user's name in the From header and email in the Reply-To when delivering email on behalf of the app's users.
• Use stubs and spies (not mocks) in isolated tests.

Some of these are protocol, such as "Have Heroku remotes named "staging" and "production."", and could be re-written as a step in a "set up project" process. For instance:

Add Heroku remotes for staging and production environments.

git remote add staging [email protected]:<app>-staging.git
git checkout -b staging --track staging/master
git remote add production [email protected]:<app>-production.git
git checkout -b production --track production/master

Some of these are new project defaults, such as "Define the project's Ruby version in the Gemfile.", and should be in Suspenders.

Some of these are code design guidelines, such as "Use stubs and spies (not mocks) in isolated tests." A lot of the move-out guidelines fall under this category. Example: "Avoid multicolumn indexes in Postgres. It combines multiple indexes efficiently."

I propose that "code design guidelines" (that we agree on) do belong in the style guide.

If you agree, rather than "cosmetic" as the heuristic for what goes in here, I propose "eliminating day-to-day decisions." We don't want to decide about formatting, flat-v-nested tests, and other things day-to-day.

I propose we move the protocol items into its own repo. Things like creating a new Rails app, feature branch code reviews, and deploys would go here.

I'm going to begin on this and submit pull requests so we can see this in practice. Would love feedback on the approach.

Regarding these two rules:

Use empty lines around multi-line blocks.

From formatting guidelines

Separate setup, exercise, verification, and teardown phases with newlines.

From testing guidelines.

I don't think it makes sense to mix them in tests.

The reason for the second guideline is to make it easy, at a glance, to distinguish the test phases (setup, execution, verification). Combine that with padding trivial blocks with newlines, and the benefit of the second guideline is greatly diminished.

Imagine a scenario where you have a test like the screenshot below. If we apply the squint/blur test to simulate at-a-glance identification of test phases, we can see that it's much harder to see the test phases in the second test; thus, unnecessary confusion!

I understand that wrapping blocks with newlines can help with code esthetics. However, identifying a test phase at a glance is more important, IMO, than a subjective esthetic.

Does it make sense to clarify the rule of "Use empty lines around multi-line blocks."? On the other hand, maybe it's worth removing it unless there a practical reason to have that guideline?

This applies to Ruby and Javascript.

New Relic, etc.

* Create a read-only [Heroku Follower](http://goo.gl/xWDMx) for your
  production database. If a Heroku database outage occurs, Heroku can use the
  follower to get your app back up and running faster.

Hi there,

I really appreciate the guidelines that you've put together here and I'm wondering if you have any thoughts on how to organize keyframes and animation transitions in SASS. Should keyframe definitions exist inline, or be kept separately? Would you consider moving all transition properties into an animations file?