Merge pull request #1 from thoughtbot/master

merging with source

Author: WebDevJL

.hound.yml

@@ -0,0 +1,11 @@
+javascript:
+  enabled: false
+jscs:
+  enabled: true
+  config_file: style/javascript/.jscsrc
+ruby:
+  enabled: true
+  config_file: style/ruby/.rubocop.yml
+scss:
+  enabled: true
+  config_file: style/sass/.scss-lint.yml

README.md

@@ -3,13 +3,17 @@ Guides
 
 Guides for getting things done, programming well, and programming in style.
 
+* [Best Practices](/best-practices)
+* [Code Review](/code-review)
+* [How to](/how-to)
 * [Protocol](/protocol)
+  * [Communication](/protocol/communication)
   * [Git](/protocol/git)
-  * [Rails](/protocol/rails)
   * [iOS](/protocol/ios)
   * [Open Source](/protocol/open-source)
-* [Code Review](/code-review)
-* [Best Practices](/best-practices)
+  * [Product Review](/protocol/product-review)
+  * [Rails](/protocol/rails)
+* [Security](/security)
 * [Style](/style)
 
 High level guidelines:

best-practices/README.md

@@ -11,10 +11,9 @@ General
 * Don't duplicate the functionality of a built-in library.
 * Don't swallow exceptions or "fail silently."
 * Don't write code that guesses at future functionality.
-* [Exceptions should be exceptional].
+* Exceptions should be exceptional.
 * [Keep the code simple].
 
-[Exceptions should be exceptional]: http://www.readability.com/~/yichhgvu
 [Keep the code simple]: http://www.readability.com/~/ko2aqda2
 
 Object-Oriented Design
@@ -64,6 +63,7 @@ Ruby Gems
 Rails
 -----
 
+* [Add foreign key constraints][fkey] in migrations.
 * Avoid bypassing validations with methods like `save(validate: false)`,
   `update_attribute`, and `toggle`.
 * Avoid instantiating more than one object in controllers.
@@ -87,6 +87,8 @@ Rails
   patch level for a project.
 * Use `_url` suffixes for named routes in mailer views and [redirects].  Use
   `_path` suffixes for named routes everywhere else.
+* Use a [class constant rather than the stringified class name][class constant in association]
+  for `class_name` options on ActiveRecord association macros.
 * Validate the associated `belongs_to` object (`user`), not the database column
   (`user_id`).
 * Use `db/seeds.rb` for data that is required in all environments.
@@ -97,17 +99,24 @@ Rails
 * Prefer `Time.zone.parse("2014-07-04 16:05:37")` over `Time.parse("2014-07-04 16:05:37")`
 * Use `ENV.fetch` for environment variables instead of `ENV[]`so that unset
   environment variables are detected on deploy.
+* [Use blocks][date-block] when declaring date and time attributes in FactoryGirl factories.
+* Use `touch: true` when declaring `belongs_to` relationships.
 
+[date-block]: /best-practices/samples/ruby.rb#L10
+[fkey]: http://robots.thoughtbot.com/referential-integrity-with-foreign-keys
 [`.ruby-version`]: https://gist.github.com/fnichol/1912050
 [redirects]: http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.30
 [Spring binstubs]: https://github.com/sstephenson/rbenv/wiki/Understanding-binstubs
 [prevent tampering]: http://blog.bigbinary.com/2013/03/19/cookies-on-rails.html
+[class constant in association]: https://github.com/thoughtbot/guides/blob/master/style/rails/sample.rb
 
 Testing
 -------
 
 * Avoid `any_instance` in rspec-mocks and mocha. Prefer [dependency injection].
-* Avoid `its`, `let`, `let!`, `specify`, and `before` in RSpec.
+* Avoid `its`, `specify`, and `before` in RSpec.
+* Avoid `let` (or `let!`) in RSpec. Prefer extracting helper methods,
+  but do not re-implement the functionality of `let`. [Example][avoid-let].
 * Avoid using `subject` explicitly *inside of an* RSpec `it` block.
   [Example][subject-example].
 * Avoid using instance variables in tests.
@@ -125,7 +134,8 @@ Testing
 * Use non-[SUT] methods in expectations when possible.
 
 [dependency injection]: http://en.wikipedia.org/wiki/Dependency_injection
-[explicit subject example]: /style/samples/testing.rb#17
+[subject-example]: ../style/testing/unit_test_spec.rb
+[avoid-let]: ../style/testing/avoid_let_spec.rb
 [`Delayed::Job` matcher]: https://gist.github.com/3186463
 [stubs and spies]: http://robots.thoughtbot.com/post/159805295/spy-vs-spy
 [assertions about state]: https://speakerdeck.com/skmetz/magic-tricks-of-testing-railsconf?slide=51
@@ -155,6 +165,9 @@ Postgres
 * Consider a [partial index] for queries on booleans.
 * Constrain most columns as [`NOT NULL`].
 * [Index foreign keys].
+* Use an `ORDER BY` clause on queries where the results will be displayed to a
+  user, as queries without one may return results in a changing, arbitrary
+  order.
 
 [`NOT NULL`]: http://www.postgresql.org/docs/9.1/static/ddl-constraints.html#AEN2444
 [combines multiple indexes]: http://www.postgresql.org/docs/9.1/static/indexes-bitmap-scans.html
@@ -173,17 +186,28 @@ Email
 
 * Use [SendGrid] or [Amazon SES] to deliver email in staging and production
   environments.
-* Use a tool like [MailView] to look at each created or updated mailer view
-  before merging.
+* Use a tool like [ActionMailer Preview] to look at each created or updated mailer view
+  before merging. Use [MailView] gem unless using Rails version 4.1.0 or later.
 
 [Amazon SES]: http://robots.thoughtbot.com/post/3105121049/delivering-email-with-amazon-ses-in-a-rails-3-app
 [SendGrid]: https://devcenter.heroku.com/articles/sendgrid
 [MailView]: https://github.com/37signals/mail_view
+[ActionMailer Preview]: http://api.rubyonrails.org/v4.1.0/classes/ActionMailer/Base.html#class-ActionMailer::Base-label-Previewing+emails
+
+Web
+---
+
+* Avoid a Flash of Unstyled Text, even when no cache is available.
+* Avoid rendering delays caused by synchronous loading.
+* Use https instead of http when linking to assets.
 
 JavaScript
 ----------
+* Use the latest stable JavaScript syntax with a transpiler, such as [babel].
+* Include a `to_param` or `href` attribute when serializing ActiveRecord models,
+  and use that when constructing URLs client side, rather than the ID.
 
-* Use CoffeeScript.
+[babel]: http://babeljs.io/
 
 HTML
 ----
@@ -196,6 +220,10 @@ CSS
 ---
 
 * Use Sass.
+* Use [Autoprefixer][autoprefixer] to generate vendor prefixes based on the
+  project-specific browser support that is needed.
+
+[autoprefixer]: https://github.com/postcss/autoprefixer
 
 Sass
 ----
@@ -209,8 +237,7 @@ Sass
 Browsers
 --------
 
-* Don't support clients without Javascript.
-* Don't support IE6 or IE7.
+* Avoid supporting versions of Internet Explorer before IE10.
 
 Objective-C
 -----------
@@ -291,6 +318,20 @@ Ember
 * Don't use jQuery outside of views and components.
 * Prefer to use predefined `Ember.computed.*` functions when possible.
 * Use `href="#"` for links that have an action.
+* Prefer dependency injection through `Ember.inject` over initializers, globals
+  on window, or namespaces. ([sample][inject])
+* Prefer sub-routes over maintaining state.
+* Prefer explicit setting of boolean properties over `toggleProperty`.
+* Prefer testing your application with [QUnit][ember-test-guides].
+
+[ember-test-guides]: https://guides.emberjs.com/v2.2.0/testing/
+
+Testing
+
+* Prefer `findWithAssert` over `find` when fetching an element you expect to
+  exist
+
+[inject]: samples/ember.js#L1-L11
 
 Angular
 -------
@@ -307,3 +348,18 @@ Angular
 [annotations]: http://robots.thoughtbot.com/avoid-angularjs-dependency-annotation-with-rails
 [ngannotate]: https://github.com/kikonen/ngannotate-rails
 [angular-translate]: https://github.com/angular-translate/angular-translate/wiki/Getting-Started#using-translate-directive
+
+Ruby JSON APIs
+--------------
+
+* Review the recommended practices outlined in Heroku's [HTTP API Design Guide]
+  before designing a new API.
+* Use a fast JSON parser, e.g. [`oj`][oj]
+* Write integration tests for your API endpoints. When the primary consumer of
+  the API is a JavaScript client maintained within the same code base as the
+  provider of the API, write [feature specs]. Otherwise write [request specs].
+
+[HTTP API Design Guide]: https://github.com/interagent/http-api-design
+[oj]: https://github.com/ohler55/oj
+[feature specs]: https://www.relishapp.com/rspec/rspec-rails/docs/feature-specs/feature-spec
+[request specs]: https://www.relishapp.com/rspec/rspec-rails/docs/request-specs/request-spec

best-practices/samples/ember.js

@@ -0,0 +1,11 @@
+import Ember from 'ember';
+
+const { inject } = Ember;
+
+export default Ember.Controller({
+  applicationController: inject.controller('application'),
+  index: inject.controller(),
+
+  toastService: inject.service('toast'),
+  store: inject.service(),
+});

best-practices/samples/handlebars.hbs

@@ -0,0 +1,5 @@
+<!-- Use toParam when constructing URLs, not ID -->
+<!-- Good -->
+<a href="/posts/{{ post.toParam }}">
+<!-- Bad -->
+<a href="/posts/{{ post.id }}">

best-practices/samples/ruby.rb

@@ -0,0 +1,12 @@
+# Include an href or to_param attribute when serializing models
+class PostSerializer < ActiveModel::Serializer
+  attributes :id, :content, :to_param
+
+  delegate :to_param, to: :object
+end
+
+FactoryGirl.define do
+  factory :event do
+    start_on { 1.week.from_now }
+  end
+end

code-review/README.md

@@ -46,7 +46,8 @@ Having Your Code Reviewed
 Reviewing Code
 --------------
 
-Understand why the code is necessary (bug, user experience, refactoring). Then:
+Understand why the change is necessary (fixes a bug, improves the user
+experience, refactors the existing code). Then:
 
 * Communicate which ideas you feel strongly about and those you don't.
 * Identify ways to simplify the code while still solving the problem.

how-to/README.rst

@@ -0,0 +1,55 @@
+How do I ...
+============
+
+.. contents::
+
+... start a new Rails app?
+--------------------------
+
+Use suspenders_:
+
+.. code:: sh
+
+  $ gem install suspenders
+  $ suspenders the-name-of-your-project-here
+  $ cd the-name-of-your-project-here/
+  $ bin/setup
+  $ rake
+
+.. _suspenders: https://github.com/thoughtbot/suspenders
+
+... feature-test a Rails app's Javascript?
+------------------------------------------
+
+Use capybara-webkit_. In your ``Gemfile``:
+
+.. code:: ruby
+
+  gem "capybara-webkit"
+
+In ``spec/support/capybara_webkit.rb`` (for Rspec):
+
+.. code:: ruby
+
+  Capybara.javascript_driver = :webkit
+  
+  Capybara::Webkit.configure do |config|
+    config.block_unknown_urls
+  end
+
+When writing a spec, you must set the ``:js`` flag for that test to make use of
+capybara-webkit. For example, in ``spec/features/user_signs_in_spec.rb``:
+
+.. code:: ruby
+
+  feature "Authentication", :js do
+    scenario "A user signing in" do
+      create(:user, email: "[email protected]", password: "sekrit")
+  
+      sign_in_as email: "[email protected]", password: "sekrit"
+  
+      expect(page).to have_text("Welcome!")
+    end
+  end
+
+.. _capybara-webkit: https://github.com/thoughtbot/capybara-webkit

protocol/README.md

@@ -5,6 +5,7 @@ Guides for getting things done.
 
 * [Communication](/protocol/communication)
 * [Git](/protocol/git)
-* [Rails](/protocol/rails)
 * [iOS](/protocol/ios)
 * [Open Source](/protocol/open-source)
+* [Product Review](/protocol/product-review)
+* [Rails](/protocol/rails)

protocol/git/README.md

@@ -24,8 +24,6 @@ Create a local feature branch based off master.
     git pull
     git checkout -b <branch-name>
 
-Prefix the branch name with your initials.
-
 Rebase frequently to incorporate upstream changes.
 
     git fetch origin
@@ -49,8 +47,9 @@ Write a [good commit message]. Example format:
 
     http://project.management-system.com/ticket/123
 
-If you've created more than one commit, use a rebase to squash them into
-cohesive commits with good messages:
+If you've created more than one commit,
+[use `git rebase` interactively](https://help.github.com/articles/about-git-rebase/)
+to squash them into cohesive commits with good messages:
 
     git rebase -i origin/master
 
@@ -100,7 +99,7 @@ Force push your branch. This allows GitHub to automatically close your pull
 request and mark it as merged when your commit(s) are pushed to master. It also
  makes it possible to [find the pull request] that brought in your changes.
 
-    git push --force origin <branch-name>
+    git push --force-with-lease origin <branch-name>
 
 View a list of new commits. View changed files. Merge branch into master.