From e3292123006a82cf0e95a97821fc94947f00604d Mon Sep 17 00:00:00 2001 From: Damian Galarza Date: Tue, 21 Jan 2014 15:03:52 -0500 Subject: [PATCH 001/488] Add guideline to order i18n keys alphabetically Ordering Rails i18n keys alphabetically makes the translations file easier to navigate as well as making merge conflicts easier to deal with. --- style/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/style/README.md b/style/README.md index 7c246116..bbb89880 100644 --- a/style/README.md +++ b/style/README.md @@ -176,6 +176,7 @@ Rails * Order ActiveRecord associations alphabetically by attribute name. * Order ActiveRecord validations alphabetically by attribute name. * Order controller contents: filters, public methods, private methods. +* Order i18n translations alphabetically by key name. * Order model contents: constants, macros, public methods, private methods. * Put application-wide partials in the [`app/views/application`] directory. * Use `def self.method`, not the `scope :method` DSL. From 9baa8a38b34c6017062192305d7bab69a73025c0 Mon Sep 17 00:00:00 2001 From: Adarsh Pandit Date: Mon, 27 Jan 2014 01:31:25 -0800 Subject: [PATCH 002/488] Update copyright to 2014 --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index fc294469..845f4864 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,7 @@ Guides is maintained by [thoughtbot, inc](http://thoughtbot.com/community). License ------- -Guides is © 2013 thoughtbot, inc. It is distributed under the [Creative Commons +Guides is © 2014 thoughtbot, inc. It is distributed under the [Creative Commons Attribution License](http://creativecommons.org/licenses/by/3.0/). The names and logos for thoughtbot are trademarks of thoughtbot, inc. From f3678329bc83bce21780e6e3658bef2c086257fb Mon Sep 17 00:00:00 2001 From: Adarsh Pandit Date: Tue, 21 Jan 2014 10:02:34 -0800 Subject: [PATCH 003/488] Add more detail to type naming guidance --- style/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/style/README.md b/style/README.md index bbb89880..53ee2a2d 100644 --- a/style/README.md +++ b/style/README.md @@ -44,7 +44,7 @@ Naming ------ * Avoid abbreviations. -* Avoid types in names (`user_array`). +* Avoid object types in names (`user_array`, `email_method` `CalculatorClass`, `ReportModule`). * Name the enumeration parameter the singular of the collection. * Name variables, methods, and classes to reveal intent. * Treat acronyms as words in names (`XmlHttpRequest` not `XMLHTTPRequest`), From eb024567d5cb7eed7cfeeac122f133b239d4f3ed Mon Sep 17 00:00:00 2001 From: Joe Ferris Date: Wed, 12 Feb 2014 14:33:57 -0500 Subject: [PATCH 004/488] Prefer rescue without begin/end * Less boilerplate * Makes it more obvious when a new method can be extracted --- style/README.md | 1 + style/samples/ruby.rb | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/style/README.md b/style/README.md index 53ee2a2d..bffaff69 100644 --- a/style/README.md +++ b/style/README.md @@ -149,6 +149,7 @@ Ruby * Use `def self.method`, not `def Class.method` or `class << self`. * Use `def` with parentheses when there are arguments. * Use `each`, not `for`, for iteration. +* Use `rescue` without `begin`/`end` when possible. * Use heredocs for multi-line strings. ERb diff --git a/style/samples/ruby.rb b/style/samples/ruby.rb index 586977e6..94aba4b6 100644 --- a/style/samples/ruby.rb +++ b/style/samples/ruby.rb @@ -72,6 +72,12 @@ def method_that_uses_factory user.ensure_authenticated! end + def method_which_raises_an_exception + behavior_that_might_raise + rescue ExceptionClass => exception + handle_exception exception + end + def self.class_method method_body end From 7fea88142bf2d6cc9aacee6ba7b7ea72dc0e4a46 Mon Sep 17 00:00:00 2001 From: Mike Burns Date: Fri, 14 Feb 2014 16:22:21 +0100 Subject: [PATCH 005/488] Remain undecided on begin/rescue/end In #135 this remained a controversial decision, with some arguing that the code becomes difficult to read without `begin`, others arguing that it makes it too hard to see what the raising method call is. Since we are still undecided on this, we can revert this decision. This reverts commit eb024567d5cb7eed7cfeeac122f133b239d4f3ed. --- style/README.md | 1 - style/samples/ruby.rb | 6 ------ 2 files changed, 7 deletions(-) diff --git a/style/README.md b/style/README.md index bffaff69..53ee2a2d 100644 --- a/style/README.md +++ b/style/README.md @@ -149,7 +149,6 @@ Ruby * Use `def self.method`, not `def Class.method` or `class << self`. * Use `def` with parentheses when there are arguments. * Use `each`, not `for`, for iteration. -* Use `rescue` without `begin`/`end` when possible. * Use heredocs for multi-line strings. ERb diff --git a/style/samples/ruby.rb b/style/samples/ruby.rb index 94aba4b6..586977e6 100644 --- a/style/samples/ruby.rb +++ b/style/samples/ruby.rb @@ -72,12 +72,6 @@ def method_that_uses_factory user.ensure_authenticated! end - def method_which_raises_an_exception - behavior_that_might_raise - rescue ExceptionClass => exception - handle_exception exception - end - def self.class_method method_body end From 11e2d272f4d655df8eefb1e57d55e55b6a83d9c7 Mon Sep 17 00:00:00 2001 From: Blaine Schmeisser Date: Wed, 19 Feb 2014 10:00:13 -0600 Subject: [PATCH 006/488] Remove triple equals operator in coffee styles The triple equals operator does not exist in coffeescript and will not compile with it. --- style/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/style/README.md b/style/README.md index 53ee2a2d..1307974c 100644 --- a/style/README.md +++ b/style/README.md @@ -116,7 +116,7 @@ CoffeeScript * Use `PascalCase` for classes, `lowerCamelCase` for variables and functions, `SCREAMING_SNAKE_CASE` for constants, `_single_leading_underscore` for private variables and functions. -* Prefer `is` to `== ` or `===` +* Prefer `is` to `== ` * Prefer `or` and `and` to `||` and `&&` Ruby From 6440772b9d685238ee6b793def4ed5de7bf2f6db Mon Sep 17 00:00:00 2001 From: Damian Galarza Date: Wed, 19 Feb 2014 10:47:00 -0500 Subject: [PATCH 007/488] Avoid trailing conditionals in CoffeeScript The same reasons for avoiding trailing conditionals in Ruby apply to CoffeeScript. Therefore, we should avoid them in CoffeeScript too. --- style/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/style/README.md b/style/README.md index 1307974c..0c375d00 100644 --- a/style/README.md +++ b/style/README.md @@ -110,6 +110,7 @@ Sass CoffeeScript ------------ +* Avoid conditional modifiers (lines that end with conditionals). * Initialize arrays using `[]`. * Initialize empty objects and hashes using `{}`. * Use hyphen-separated filenames, such as `coffee-script.coffee`. From 8872a539e0c07099f277ec7de55c168a1b6ac185 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Quenneville?= Date: Fri, 24 Jan 2014 10:27:16 -0500 Subject: [PATCH 008/488] Use `should` shorthand for one-liners According to the RSpec source: > The one-liner syntax supported by rspec-core uses should even when > `config.syntax = :expect`. It reads better than the alternative, and > does not require a global monkey patch This is especially the case when working with `shoulda-matchers` --- style/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/style/README.md b/style/README.md index 0c375d00..13b71c79 100644 --- a/style/README.md +++ b/style/README.md @@ -217,9 +217,11 @@ Testing * Prefer `eq` to `==` in RSpec. * Separate setup, exercise, verification, and teardown phases with newlines. * Use RSpec's [`expect` syntax]. +* Use `should` shorthand for [one-liners with an implicit subject]. * Use `not_to` instead of `to_not` in RSpec expectations. [`expect` syntax]: http://myronmars.to/n/dev-blog/2012/06/rspecs-new-expectation-syntax +[one-liners with an implicit subject]: https://github.com/rspec/rspec-expectations/blob/master/Should.md#one-liners #### Acceptance Tests From a9273ca3098a23df65a8dadccae8a63ede7169d5 Mon Sep 17 00:00:00 2001 From: Greg Lazarev Date: Fri, 28 Feb 2014 13:39:32 -0800 Subject: [PATCH 009/488] Mention "Imperative mood" for `it` descriptions --- style/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/style/README.md b/style/README.md index 13b71c79..30773588 100644 --- a/style/README.md +++ b/style/README.md @@ -251,7 +251,8 @@ Testing [Sample](samples/testing.rb) -* Don't prefix `it` block descriptions with 'should'. +* Don't prefix `it` block descriptions with `should`. Use + [Imperative mood](http://en.wikipedia.org/wiki/Imperative_mood) instead. * Name outer `describe` blocks after the method under test. Use `.method` for class methods and `#method` for instance methods. From 2ccac0ceb553ffb5854bd61eb60958a9b12c024f Mon Sep 17 00:00:00 2001 From: Dan Croak Date: Fri, 21 Feb 2014 09:52:46 -0800 Subject: [PATCH 010/488] Split protocol document into smaller documents * Extract Git, Rails, and iOS protocols. * Move Rails-specific code review to Rails protocol. * Make git protocol less Rails-specific (instead of `rake`, say "run tests" so it can continue to apply to iOS, Haskell, etc. projects). * Nest bullets. * Use consistent capitalization. --- README.md | 3 + code-review/README.md | 21 +--- protocol/README.md | 226 +----------------------------------------- protocol/git.md | 109 ++++++++++++++++++++ protocol/ios.md | 39 ++++++++ protocol/rails.md | 121 ++++++++++++++++++++++ 6 files changed, 279 insertions(+), 240 deletions(-) create mode 100644 protocol/git.md create mode 100644 protocol/ios.md create mode 100644 protocol/rails.md diff --git a/README.md b/README.md index 845f4864..b2314b3e 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,9 @@ Guides Guides for getting things done, programming well, and programming in style. * [Protocol](/protocol) + * [Git](/protocol/git) + * [Rails](/protocol/rails) + * [iOS](/protocol/ios) * [Code Review](/code-review) * [Best Practices](/best-practices) * [Style](/style) diff --git a/code-review/README.md b/code-review/README.md index e7e7f05c..e9842a6b 100644 --- a/code-review/README.md +++ b/code-review/README.md @@ -23,7 +23,7 @@ Everyone * Talk in person if there are too many "I didn't understand" or "Alternative solution:" comments. Post a follow-up comment summarizing offline discussion. -Having your code reviewed +Having Your Code Reviewed ------------------------- * Be grateful for the reviewer's suggestions. ("Good call. I'll make that @@ -43,7 +43,7 @@ Having your code reviewed * Wait to merge the branch until Continuous Integration (TDDium, TravisCI, etc.) tells you the test suite is green in the branch. -Reviewing code +Reviewing Code -------------- Understand why the code is necessary (bug, user experience, refactoring). Then: @@ -58,7 +58,7 @@ Understand why the code is necessary (bug, user experience, refactoring). Then: * Seek to understand the author's perspective. * Sign off on the pull request with a :thumbsup: or "Ready to merge" comment. -Style comments +Style Comments -------------- Reviewers should comment on missed [style](../style) @@ -74,18 +74,3 @@ An example response to style comments: If you disagree with a guideline, open an issue on the guides repo rather than debating it within the code review. In the meantime, apply the guideline. - -Ruby on Rails review --------------------- - -* Review data integrity closely, such as migrations that make irreversible - changes to the data, and whether there is a related todo to make a database - backup during the staging and production deploys. -* Review SQL queries for potential SQL injection. -* Review whether dependency upgrades include a reason in the commit message, - such as a link to the dependency's `ChangeLog` or `NEWS` file. -* Review whether new database indexes are necessary if new columns or SQL - queries were added. -* Review whether new scheduler (`cron`) tasks have been added and whether there - is a related todo in the project management system to add it during the - staging and production deploys. diff --git a/protocol/README.md b/protocol/README.md index b1418fd8..04a4439c 100644 --- a/protocol/README.md +++ b/protocol/README.md @@ -1,226 +1,8 @@ Protocol ======== -A guide for getting things done. +Guides for getting things done. -Set up laptop -------------- - -Install the latest version of Xcode from the App Store. - -Set up your laptop with [this script](https://github.com/thoughtbot/laptop) -and [these dotfiles](https://github.com/thoughtbot/dotfiles). - -Create Rails app ----------------- - -Get Suspenders. - - gem install suspenders - -Create the app. - - suspenders app --heroku true --github organization/app - -Create iOS app --------------- - -Create a new project in Xcode with these settings: - -* Check 'Create local git repository for this project'. -* Check 'Use Automatic Reference Counting'. -* Set an appropriate 2 or 3 letter class prefix. -* Set the Base SDK to 'Latest iOS'. -* Set the iOS Deployment Target to 6.0. -* Use the Apple LLVM compiler. - -Get liftoff. - - gem install liftoff - -Run liftoff in the project directory. - - liftoff - -Set up Rails app ----------------- - -Get the code. - - git clone git@github.com:organization/app.git - -Set up the app's dependencies. - - cd project - ./bin/setup - -Use [Heroku config](https://github.com/ddollar/heroku-config) to get `ENV` -variables. - - heroku config:pull --remote staging - -Delete extra lines in `.env`, leaving only those needed for app to function -properly. For example: `BRAINTREE_MERCHANT_ID` and `S3_SECRET`. - -Use [Foreman](https://github.com/ddollar/foreman) to run the app locally. - - foreman start - -It uses your `.env` file and `Procfile` to run processes just like Heroku's -[Cedar](https://devcenter.heroku.com/articles/cedar/) stack. - -Maintain a Rails app --------------------- - -* 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. -* Rebase frequently to incorporate upstream changes. -* Use a [pull request] for code reviews. - -[pull request]: https://help.github.com/articles/using-pull-requests/ - -Write a feature ---------------- - -Create a local feature branch based off master. - - git checkout master - git pull - git checkout -b - -Prefix the branch name with your initials. - -Rebase frequently to incorporate upstream changes. - - git fetch origin - git rebase origin/master - -Resolve conflicts. When feature is complete and tests pass, stage the changes. - - rake - git add --all - -When you've staged the changes, commit them. - - git status - git commit --verbose - -Write a [good commit message]. Example format: - - Present-tense summary under 50 characters - - * More information about commit (under 72 characters). - * More information about commit (under 72 characters). - - http:://project.management-system.com/ticket/123 - -Share your branch. - - git push origin - -Submit a [GitHub pull request]. - -Ask for a code review in [Campfire](https://campfirenow.com/). - -[good commit message]: http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html -[GitHub pull request]: https://help.github.com/articles/using-pull-requests/ - -Review code ------------ - -A team member other than the author reviews the pull request. They follow -[Code Review](../code-review) guidelines to avoid -miscommunication. - -They make comments and ask questions directly on lines of code in the Github -web interface or in Campfire. - -For changes which they can make themselves, they check out the branch. - - git checkout - rake db:migrate - rake - git diff staging/master..HEAD - -They make small changes right in the branch, test the feature in browser, -run tests, commit, and push. - -When satisfied, they comment on the pull request `Ready to merge.` - -Merge ------ - -Rebase interactively. Squash commits like "Fix whitespace" into one or a -small number of valuable commit(s). Edit commit messages to reveal intent. - - git fetch origin - git rebase -i origin/master - rake - -View a list of new commits. View changed files. Merge branch into master. - - git log origin/master.. - git diff --stat origin/master - git checkout master - git merge --ff-only - git push - -Delete your remote feature branch. - - git push origin --delete - -Delete your local feature branch. - - git branch --delete - -Deploy ------- - -View a list of new commits. View changed files. Deploy to -[Heroku](https://devcenter.heroku.com/articles/quickstart) staging. - - git fetch staging - git log staging/master..master - git diff --stat staging/master - git push staging - -If necessary, run migrations and restart the dynos. - - heroku run rake db:migrate --remote staging - heroku restart --remote staging - -[Introspect] to make sure everything's ok. - - watch heroku ps --remote staging - -Test the feature in browser. - -Deploy to production. - - git fetch production - git log production/master..master - git diff --stat production/master - git push production - heroku run rake db:migrate --remote production - heroku restart --remote production - watch heroku ps --remote production - -Watch logs and metrics dashboards. - -Close pull request and comment `Merged.` - -[Introspect]: http://blog.heroku.com/archives/2011/6/24/the_new_heroku_3_visibility_introspection/ - -Set Up Production Environment ------------------------------ - -* Make sure that your [`Procfile`] is set up to run Unicorn. -* Make sure the PG Backups add-on is enabled. -* Create a read-only [Heroku Follower] for your production database. If a Heroku - database outage occurs, Heroku can use the follower to get your app back up - and running faster. - -[Heroku Follower]: https://devcenter.heroku.com/articles/improving-heroku-postgres-availability-with-followers -[`Procfile`]: https://devcenter.heroku.com/articles/procfile +* [Git](/protocol/git) +* [Rails](/protocol/rails) +* [iOS](/protocol/ios) diff --git a/protocol/git.md b/protocol/git.md new file mode 100644 index 00000000..bbe7dc91 --- /dev/null +++ b/protocol/git.md @@ -0,0 +1,109 @@ +Git Protocol +============ + +A guide for programming within version control. + +Maintain a Repo +--------------- + +* 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. +* Rebase frequently to incorporate upstream changes. +* Use a [pull request] for code reviews. + +[pull request]: https://help.github.com/articles/using-pull-requests/ + +Write a Feature +--------------- + +Create a local feature branch based off master. + + git checkout master + git pull + git checkout -b + +Prefix the branch name with your initials. + +Rebase frequently to incorporate upstream changes. + + git fetch origin + git rebase origin/master + +Resolve conflicts. When feature is complete and tests pass, stage the changes. + + git add --all + +When you've staged the changes, commit them. + + git status + git commit --verbose + +Write a [good commit message]. Example format: + + Present-tense summary under 50 characters + + * More information about commit (under 72 characters). + * More information about commit (under 72 characters). + + http:://project.management-system.com/ticket/123 + +Share your branch. + + git push origin + +Submit a [GitHub pull request]. + +Ask for a code review in the project's chat room. + +[good commit message]: http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html +[GitHub pull request]: https://help.github.com/articles/using-pull-requests/ + +Review Code +----------- + +A team member other than the author reviews the pull request. They follow +[Code Review](/code-review) guidelines to avoid +miscommunication. + +They make comments and ask questions directly on lines of code in the GitHub +web interface or in the project's chat room. + +For changes which they can make themselves, they check out the branch. + + git checkout + ./bin/setup + git diff staging/master..HEAD + +They make small changes right in the branch, test the feature on their machine, +run tests, commit, and push. + +When satisfied, they comment on the pull request `Ready to merge.` + +Merge +----- + +Rebase interactively. Squash commits like "Fix whitespace" into one or a +small number of valuable commit(s). Edit commit messages to reveal intent. Run +tests. + + git fetch origin + git rebase -i origin/master + +View a list of new commits. View changed files. Merge branch into master. + + git log origin/master.. + git diff --stat origin/master + git checkout master + git merge --ff-only + git push + +Delete your remote feature branch. + + git push origin --delete + +Delete your local feature branch. + + git branch --delete + diff --git a/protocol/ios.md b/protocol/ios.md new file mode 100644 index 00000000..8a881fe0 --- /dev/null +++ b/protocol/ios.md @@ -0,0 +1,39 @@ +iOS Protocol +============ + +A guide for making iPhone and iPad apps with aplomb. + +Set Up Laptop +------------- + +Install the latest version of Xcode from the App Store. + +Create App +---------- + +Create a new project in Xcode with these settings: + +* Check 'Create local git repository for this project'. +* Check 'Use Automatic Reference Counting'. +* Set an appropriate 2 or 3 letter class prefix. +* Set the Base SDK to 'Latest iOS'. +* Set the iOS Deployment Target to 6.0. +* Use the Apple LLVM compiler. + +Get liftoff. + + gem install liftoff + +Run liftoff in the project directory. + + liftoff + +Git Protocol +------------ + +Follow the normal [Git Protocol](/protocol/git). + +Code Review +----------- + +Follow the normal [Code Review guidelines](/code-review). diff --git a/protocol/rails.md b/protocol/rails.md new file mode 100644 index 00000000..c331b97a --- /dev/null +++ b/protocol/rails.md @@ -0,0 +1,121 @@ +Rails Protocol +============== + +A guide for writing great web apps. + +Set Up Laptop +------------- + +Set up your laptop with [this script](https://github.com/thoughtbot/laptop) +and [these dotfiles](https://github.com/thoughtbot/dotfiles). + +Create App +---------- + +Get Suspenders. + + gem install suspenders + +Create the app. + + suspenders app --heroku true --github organization/app + +Set Up App +---------- + +Get the code. + + git clone git@github.com:organization/app.git + +Set up the app's dependencies. + + cd project + ./bin/setup + +Use [Heroku config](https://github.com/ddollar/heroku-config) to get `ENV` +variables. + + heroku config:pull --remote staging + +Delete extra lines in `.env`, leaving only those needed for app to function +properly. For example: `BRAINTREE_MERCHANT_ID` and `S3_SECRET`. + +Use [Foreman](https://github.com/ddollar/foreman) to run the app locally. + + foreman start + +It uses your `.env` file and `Procfile` to run processes just like Heroku's +[Cedar](https://devcenter.heroku.com/articles/cedar/) stack. + +Git Protocol +------------ + +Follow the normal [Git Protocol](/protocol/git). + +Code Review +----------- + +Follow the normal [Code Review guidelines](/code-review). When reviewing others' +Rails work, look in particular for: + +* Review data integrity closely, such as migrations that make irreversible + changes to the data, and whether there is a related todo to make a database + backup during the staging and production deploys. +* Review SQL queries for potential SQL injection. +* Review whether dependency upgrades include a reason in the commit message, + such as a link to the dependency's `ChangeLog` or `NEWS` file. +* Review whether new database indexes are necessary if new columns or SQL + queries were added. +* Review whether new scheduler (`cron`) tasks have been added and whether there + is a related todo in the project management system to add it during the + staging and production deploys. + +Deploy +------ + +View a list of new commits. View changed files. Deploy to +[Heroku](https://devcenter.heroku.com/articles/quickstart) staging. + + git fetch staging + git log staging/master..master + git diff --stat staging/master + git push staging + +If necessary, run migrations and restart the dynos. + + heroku run rake db:migrate --remote staging + heroku restart --remote staging + +[Introspect] to make sure everything's ok. + + watch heroku ps --remote staging + +Test the feature in browser. + +Deploy to production. + + git fetch production + git log production/master..master + git diff --stat production/master + git push production + heroku run rake db:migrate --remote production + heroku restart --remote production + watch heroku ps --remote production + +Watch logs and metrics dashboards. + +Close pull request and comment `Merged.` + +[Introspect]: http://blog.heroku.com/archives/2011/6/24/the_new_heroku_3_visibility_introspection/ + +Set Up Production Environment +----------------------------- + +* Make sure that your [`Procfile`] is set up to run Unicorn. +* Make sure the PG Backups add-on is enabled. +* Create a read-only [Heroku Follower] for your production database. If a Heroku + database outage occurs, Heroku can use the follower to get your app back up + and running faster. + +[Heroku Follower]: https://devcenter.heroku.com/articles/improving-heroku-postgres-availability-with-followers +[`Procfile`]: https://devcenter.heroku.com/articles/procfile From 1d6948699d85dedb9c27e20ed4b567fea902f9b7 Mon Sep 17 00:00:00 2001 From: Dan Croak Date: Mon, 3 Mar 2014 21:06:36 -0800 Subject: [PATCH 011/488] Fix URLs --- protocol/{git.md => git/README.md} | 0 protocol/{ios.md => ios/README.md} | 0 protocol/{rails.md => rails/README.md} | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename protocol/{git.md => git/README.md} (100%) rename protocol/{ios.md => ios/README.md} (100%) rename protocol/{rails.md => rails/README.md} (100%) diff --git a/protocol/git.md b/protocol/git/README.md similarity index 100% rename from protocol/git.md rename to protocol/git/README.md diff --git a/protocol/ios.md b/protocol/ios/README.md similarity index 100% rename from protocol/ios.md rename to protocol/ios/README.md diff --git a/protocol/rails.md b/protocol/rails/README.md similarity index 100% rename from protocol/rails.md rename to protocol/rails/README.md From a30a515ff6543b9d0f8d035439811b9930cd2373 Mon Sep 17 00:00:00 2001 From: Dan Croak Date: Fri, 21 Feb 2014 10:42:09 -0800 Subject: [PATCH 012/488] Add "Open Source Protocol" --- README.md | 1 + protocol/README.md | 1 + protocol/open-source/README.md | 15 +++++++++++++++ 3 files changed, 17 insertions(+) create mode 100644 protocol/open-source/README.md diff --git a/README.md b/README.md index b2314b3e..68b46769 100644 --- a/README.md +++ b/README.md @@ -7,6 +7,7 @@ Guides for getting things done, programming well, and programming in style. * [Git](/protocol/git) * [Rails](/protocol/rails) * [iOS](/protocol/ios) + * [Open Source](/protocol/open-source) * [Code Review](/code-review) * [Best Practices](/best-practices) * [Style](/style) diff --git a/protocol/README.md b/protocol/README.md index 04a4439c..8f38e37b 100644 --- a/protocol/README.md +++ b/protocol/README.md @@ -6,3 +6,4 @@ Guides for getting things done. * [Git](/protocol/git) * [Rails](/protocol/rails) * [iOS](/protocol/ios) +* [Open Source](/protocol/open-source) diff --git a/protocol/open-source/README.md b/protocol/open-source/README.md new file mode 100644 index 00000000..d61fbe2c --- /dev/null +++ b/protocol/open-source/README.md @@ -0,0 +1,15 @@ +Open Source Protocol +==================== + +A guide for releasing and maintaining open source projects. + +Releasing a Ruby Gem +-------------------- + +* Edit the `VERSION` constant. +* Run `bundle install` to update `Gemfile.lock`. +* Run the test suite. +* Edit `NEWS`, `Changelog`, or `README` files if relevant. +* Commit changes like "Bump to 2.1.0". +* Run `rake release`, which tags the release, pushes the tag + to GitHub, and pushes the gem to Rubygems.org. From f3a31e2e8c54a42ba719a3ad8501613a63aecae6 Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Fri, 28 Feb 2014 14:28:10 -0500 Subject: [PATCH 013/488] Update style guide on unit test to match our usage --- style/README.md | 14 ++++++++++---- style/samples/testing.rb | 33 ++++++++++++++++++++++++--------- 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/style/README.md b/style/README.md index 30773588..99ef7147 100644 --- a/style/README.md +++ b/style/README.md @@ -251,10 +251,16 @@ Testing [Sample](samples/testing.rb) -* Don't prefix `it` block descriptions with `should`. Use - [Imperative mood](http://en.wikipedia.org/wiki/Imperative_mood) instead. -* Name outer `describe` blocks after the method under test. Use `.method` - for class methods and `#method` for instance methods. +* Don't prefix `it` block descriptions with `should`. Use [Imperative mood] + instead. +* Put one-liner specs at the beginning of the outer `describe` blocks. +* Use `.method` to describe class methods and `#method` to describe instance + methods. +* Use `context` to describe testing preconditions. +* Use `describe '#method_name'` to group tests by method-under-test +* Use a single, top-level `describe ClassName` block. + +[Imperative mood]: http://en.wikipedia.org/wiki/Imperative_mood Objective-C ----------- diff --git a/style/samples/testing.rb b/style/samples/testing.rb index 118ab85f..5b9164de 100644 --- a/style/samples/testing.rb +++ b/style/samples/testing.rb @@ -1,15 +1,30 @@ -describe SomeClass, '#some_method' do - it 'does something' do - expect(something).to eq 'something' - end -end +describe SomeClass do + it { should have_one(:association) } + it { should validate_presence_of(:some_attribute) } -describe SomeClass, '#other_method' do - it 'does something in one case' do + describe '.some_class_method' do + it 'does something' do + # ... + end end - it 'does something else in other cases' do + describe '#some_instance_method' do + it 'does something' do + expect(something).to eq 'something' + end end - # methods go here + describe '#another_instance_method' do + context 'when in one case' do + it 'does something' do + # ... + end + end + + context 'when in other case' do + it 'does something else' do + # ... + end + end + end end From ba67605ad01f3999862cd14f0f340ab47dd350fc Mon Sep 17 00:00:00 2001 From: Gordon Fontenot Date: Fri, 14 Mar 2014 11:24:53 -0400 Subject: [PATCH 014/488] Update iOS app creation instructions for Liftoff Also add a little more info to the Code Review section --- protocol/ios/README.md | 38 +++++++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/protocol/ios/README.md b/protocol/ios/README.md index 8a881fe0..cf91029d 100644 --- a/protocol/ios/README.md +++ b/protocol/ios/README.md @@ -11,23 +11,33 @@ Install the latest version of Xcode from the App Store. Create App ---------- -Create a new project in Xcode with these settings: +Get Liftoff. -* Check 'Create local git repository for this project'. -* Check 'Use Automatic Reference Counting'. -* Set an appropriate 2 or 3 letter class prefix. -* Set the Base SDK to 'Latest iOS'. -* Set the iOS Deployment Target to 6.0. -* Use the Apple LLVM compiler. + brew tap thoughtbot/formulae + brew install liftoff -Get liftoff. +Get CocoaPods - gem install liftoff + [sudo] gem install cocoapods -Run liftoff in the project directory. +Create the app. liftoff +* Be sure to set an appropriate 2 or 3 letter class prefix. + +Set Up App +---------- + +Get the code. + + git clone git@github.com:organization/app.git + +Install the app's dependencies. + + cd project + pod install + Git Protocol ------------ @@ -36,4 +46,10 @@ Follow the normal [Git Protocol](/protocol/git). Code Review ----------- -Follow the normal [Code Review guidelines](/code-review). +Follow the normal [Code Review guidelines](/code-review). When reviewing +others' iOS work, look in particular for: + +* Review that ViewControllers are adhering to SRP +* Watch for CoreData thread boundary violations +* Watch for potential retain cycles with blocks +* Ensure that methods that require parameters are using `NSParameterAssert()` From f5d787fa95008eb9cb7f0ba191be4887c4f39305 Mon Sep 17 00:00:00 2001 From: Joe Ferris Date: Mon, 17 Mar 2014 10:15:29 -0400 Subject: [PATCH 015/488] Guideline: Add bundler/spring binstubs to version control * Make sure each member of a team has access to binstubs * Syncronize what the stub uses (Bundler, Rails, or Spring) * Allow just one project PATH entry The rbenv wiki has an excellent guide on binstubs: https://github.com/sstephenson/rbenv/wiki/Understanding-binstubs --- best-practices/README.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/best-practices/README.md b/best-practices/README.md index e529e138..b33ec058 100644 --- a/best-practices/README.md +++ b/best-practices/README.md @@ -35,11 +35,15 @@ Ruby * Avoid optional parameters. Does the method do too much? * Avoid monkey-patching. +* Generate necessary [Bundler binstubs] for the project, such as `rake` and + `rspec`, and add them to version control. * Prefer classes to modules when designing functionality that is shared by multiple models. * Prefer `private` when indicating scope. Use `protected` only with comparison methods like `def ==(other)`, `def <(other)`, and `def >(other)`. +[Bundler binstubs](https://github.com/sstephenson/rbenv/wiki/Understanding-binstubs) + Ruby Gems --------- @@ -69,6 +73,8 @@ Rails from view templates. * Don't use SQL or SQL fragments (`where('inviter_id IS NOT NULL')`) outside of models. +* Generate necessary [Spring binstubs] for the project, such as `rake` and + `rspec`, and add them to version control. * If there are default values, set them in migrations. * Keep `db/schema.rb` or `db/development_structure.sql` under version control. * Use only one instance variable in each view. @@ -84,6 +90,7 @@ Rails [`.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 Testing ------- From dee16052f49cb4b41fb8d090ffdb75942512ddb1 Mon Sep 17 00:00:00 2001 From: Joshua Clayton Date: Wed, 12 Mar 2014 17:16:44 -0400 Subject: [PATCH 016/488] Prefer have_css to have_selector with RSpec + Capybara While have_selector supports CSS selectors and XPath selectors, have_css is most often how we assert elements are present in the DOM; as an added benefit, it's shorter to type. --- style/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/style/README.md b/style/README.md index 99ef7147..ff2cdbbb 100644 --- a/style/README.md +++ b/style/README.md @@ -219,6 +219,7 @@ Testing * Use RSpec's [`expect` syntax]. * Use `should` shorthand for [one-liners with an implicit subject]. * Use `not_to` instead of `to_not` in RSpec expectations. +* Prefer the `have_css` matcher to the `have_selector` matcher in Capybara assertions. [`expect` syntax]: http://myronmars.to/n/dev-blog/2012/06/rspecs-new-expectation-syntax [one-liners with an implicit subject]: https://github.com/rspec/rspec-expectations/blob/master/Should.md#one-liners From 237d16b2f401d0c4c4f19102fce64c69cf9e8a46 Mon Sep 17 00:00:00 2001 From: Greg Lazarev Date: Fri, 7 Mar 2014 11:16:12 -0800 Subject: [PATCH 017/488] Specify method chaining dot style rule When splitting up a chain of method calls, use a trailing `.` and place each method on its own line ```ruby foo. bar. baz ``` --- style/README.md | 4 ++++ style/samples/ruby.rb | 5 +++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/style/README.md b/style/README.md index ff2cdbbb..65a72ab1 100644 --- a/style/README.md +++ b/style/README.md @@ -29,6 +29,9 @@ Formatting brace on its own line. * Indent continued lines two spaces. * Indent private methods equal to public methods. +* 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. + [Example][dot guideline example]. * Use 2 space indentation (no tabs). * Use an empty line between methods. * Use newlines around multi-line blocks. @@ -37,6 +40,7 @@ Formatting * Use [Unix-style line endings] (`\n`). * Use [uppercase for SQL key words and lowercase for SQL identifiers]. +[dot guideline example]: https://github.com/thoughtbot/guides/blob/master/style/samples/ruby.rb#L11 [uppercase for SQL key words and lowercase for SQL identifiers]: http://www.postgresql.org/docs/9.2/static/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS [Unix-style line endings]: http://unix.stackexchange.com/questions/23903/should-i-end-my-text-script-files-with-a-newline diff --git a/style/samples/ruby.rb b/style/samples/ruby.rb index 586977e6..fc8b4a84 100644 --- a/style/samples/ruby.rb +++ b/style/samples/ruby.rb @@ -8,8 +8,9 @@ def initialize(attributes) end def method_with_arguments(argument_one, argument_two) - this_is_a_really_long_line_that_should_be_broken_up_over_multiple_lines_and. - every_line_but_the_first_is_indented + a_really_long_line_that_is_broken_up_over_multiple_lines_and. + subsequent_lines_are_indented_and. + each_method_lives_on_its_own_line end def method_with_multiline_block From 2efbd9c9378f607fc1f28597506611338164c19a Mon Sep 17 00:00:00 2001 From: Simon Taranto Date: Mon, 14 Apr 2014 15:14:13 -0700 Subject: [PATCH 018/488] change newline to emtpy line --- style/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/style/README.md b/style/README.md index 65a72ab1..d7bc94e9 100644 --- a/style/README.md +++ b/style/README.md @@ -34,7 +34,7 @@ Formatting [Example][dot guideline example]. * Use 2 space indentation (no tabs). * Use an empty line between methods. -* Use newlines around multi-line blocks. +* Use empty lines around multi-line blocks. * Use spaces around operators, after commas, after colons and semicolons, around `{` and before `}`. * Use [Unix-style line endings] (`\n`). From 234433804d0923308ae640bbeea1b9e674db5325 Mon Sep 17 00:00:00 2001 From: "Jessie A. Young" Date: Wed, 16 Apr 2014 14:27:36 -0700 Subject: [PATCH 019/488] Add migration styles * Default non-presence-validated string fields to empty string to avoid nil values * Put timestamps at the top when creating a table for consistency --- style/README.md | 17 +++++++++++++---- style/samples/migration.rb | 11 +++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) create mode 100644 style/samples/migration.rb diff --git a/style/README.md b/style/README.md index d7bc94e9..21cbf52f 100644 --- a/style/README.md +++ b/style/README.md @@ -189,6 +189,16 @@ Rails [`app/views/application`]: http://asciicasts.com/episodes/269-template-inheritance +Rails Migrations +---------------- +* Set an empty string as the default constraint for non-required string and text + fields. [Example][default example]. +* List timestamps first when creating a new table. [Example][timestamps + example]. + +[timestamps example]: /style/samples/migration.rb +[default example]: /style/samples/migration.rb#L6 + Rails Routes ------------ @@ -308,10 +318,9 @@ Shell * Break long lines on `|`, `&&`, or `||` and indent the continuations. * Don't add an extension to executable shell scripts. -* Don't put a line break before `then` or `do`, use `if ...; then` and - `while ...; do`. +* Don't put a line break before `then` or `do`, use `if ...; then` and `while + ...; do`. * Use `for x; do`, not `for x in "$@"; do`. -* Use `snake_case` for variable names and `ALLCAPS` for environment - variables. +* Use `snake_case` for variable names and `ALLCAPS` for environment variables. * Use single quotes for strings that don't contain escapes or variables. * Use two-space indentation. diff --git a/style/samples/migration.rb b/style/samples/migration.rb new file mode 100644 index 00000000..88836440 --- /dev/null +++ b/style/samples/migration.rb @@ -0,0 +1,11 @@ +class CreateClearanceUsers < ActiveRecord::Migration + def change + create_table :users do |t| + t.timestamps null: false + t.string :email, null: false + t.string :name, null: false, default: '' + end + + add_index :users, :email + end +end From 58b98cfb3df36c10fcb43f0c0e47e242b5521226 Mon Sep 17 00:00:00 2001 From: Trevor O'Reilly Date: Fri, 18 Apr 2014 09:04:28 -0400 Subject: [PATCH 020/488] Force push rebased feature branches before merging --- protocol/git/README.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/protocol/git/README.md b/protocol/git/README.md index bbe7dc91..eb95333e 100644 --- a/protocol/git/README.md +++ b/protocol/git/README.md @@ -91,6 +91,12 @@ tests. git fetch origin git rebase -i origin/master +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 + View a list of new commits. View changed files. Merge branch into master. git log origin/master.. @@ -107,3 +113,4 @@ Delete your local feature branch. git branch --delete +[find the pull request]: http://stackoverflow.com/a/17819027 From 1352764903ec78c7d39599975a20820fc06c2e14 Mon Sep 17 00:00:00 2001 From: Dan Croak Date: Mon, 21 Apr 2014 10:02:16 -0700 Subject: [PATCH 021/488] Add note about Hound automatically reviewing style --- style/README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/style/README.md b/style/README.md index 21cbf52f..0a2ec304 100644 --- a/style/README.md +++ b/style/README.md @@ -3,6 +3,11 @@ Style A guide for programming in style. +Use [Hound] to automatically review your code and comment on GitHub pull +requests for violations of the Ruby portions of this style guide. + +[Hound]: https://houndci.com + Git --- From 87f57782f622c6cd99b990c2ea4c266cfe55299f Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Tue, 8 Apr 2014 15:10:14 -0600 Subject: [PATCH 022/488] Prefer `==` to `is` in CoffeeScript Given that we prefer things like `&&` to `and` in ruby and other languages, it'd be nice to be consistent about this in CoffeeScript as well. I also find code that uses `is`, `isnt`, `and`, and `or` to be signficantly less readable. --- style/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/style/README.md b/style/README.md index 0a2ec304..dee42664 100644 --- a/style/README.md +++ b/style/README.md @@ -126,8 +126,8 @@ CoffeeScript * Use `PascalCase` for classes, `lowerCamelCase` for variables and functions, `SCREAMING_SNAKE_CASE` for constants, `_single_leading_underscore` for private variables and functions. -* Prefer `is` to `== ` -* Prefer `or` and `and` to `||` and `&&` +* Prefer `==` and `!=` to `is` and `isnt` +* Prefer `||` and `&&` to `or` and `and` Ruby ---- From 5ebfb39650fbac32661531ce35a4df133f157719 Mon Sep 17 00:00:00 2001 From: patrick brisbin Date: Thu, 17 Apr 2014 08:51:23 -0400 Subject: [PATCH 023/488] Preliminary Haskell style guide and best practices --- best-practices/README.md | 6 +++ style/README.md | 16 +++++++ style/samples/haskell.hs | 91 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+) create mode 100644 style/samples/haskell.hs diff --git a/best-practices/README.md b/best-practices/README.md index b33ec058..c194a502 100644 --- a/best-practices/README.md +++ b/best-practices/README.md @@ -253,3 +253,9 @@ In addition to Shell best practices, * Prefer `[[` over `test` or `[`. * Prefer process substitution over a pipe in `while read` loops. * Use `((` or `let`, not `$((` when you don't need the result + +Haskell +------- + +* Avoid partial functions (`head`, `read`, etc). +* Compile code with `-Wall -Werror`. diff --git a/style/README.md b/style/README.md index dee42664..f9e49886 100644 --- a/style/README.md +++ b/style/README.md @@ -329,3 +329,19 @@ Shell * Use `snake_case` for variable names and `ALLCAPS` for environment variables. * Use single quotes for strings that don't contain escapes or variables. * Use two-space indentation. + +Haskell +------- + +* Break long expressions before operators or keywords. +* Break long type signatures between arguments. +* Order imports in three sections, separating each by a blank line: + [standard libraries], third-party libraries, application modules. + Within each section, alphabetize the imports and place qualified + imports last. +* Use comma-first style exports, records, and lists. +* Use four-space indentation except the `where` keyword which is + indented two spaces [Example][where-example]. + +[standard libraries]: http://www.haskell.org/ghc/docs/latest/html/libraries/index.html +[where-example] https://github.com/thoughtbot/guides/blob/master/style/samples/haskell.hs#L46 diff --git a/style/samples/haskell.hs b/style/samples/haskell.hs new file mode 100644 index 00000000..50b18da3 --- /dev/null +++ b/style/samples/haskell.hs @@ -0,0 +1,91 @@ +module Haskell + ( SomeType(..) + , someFunction + , anotherFunction + ) where + +import Control.Monad.State (state, evalState) +import Data.List (foldl') +import Data.Text (Text) +import qualified Data.Map as M +import qualified Data.Text as T + +import Database.Persistent (runSql) +import Yesod +import Yesod.Auth (requireAuthId, maybeAuthId) + +import Foundation +import Settings + +data Colors = Red | Blue | Green + +data HTTPException + = NotFound + | NotAuthorized + | InternalServerError + | Whatever + | YouGetTheIdea + +data SomeType = SomeType + { accessorOne :: String + , accessorTwo :: String + , accessorThree :: String + } deriving Show + +someType :: SomeType +someType = SomeType + { accessorOne = "foo" + , accessorTwo = "bar" + , accessorThree = theThird + } + where + theThird :: String + theThird = "bar" + +myFunction :: Show a + => a + -> a + -> (a -> b) + -> (b, b) +myFunction = -- ... + +aList :: [String] +aList = + [ one + , two + , three + ] + +aShortFunction = let x = 7 in x + 2 + +aLongerFunction = + let x = 42 + y = 57 + z = 57 + in sum $ [x, y, z] + +aCasedFunction x = + case x of + Just y -> y + Nothing -> 0 + +aShortIf x = if x then y else z + +aLongIf xs = + if take 10 xs == [1..10] + then drop 2 $ reverse xs + else [] + +lineBreaks = take 3 -- prefix + . reverse . drop 1 . reverse . drop 1 -- strip + . map upcase -- scream + +main :: IO () +main = do + let x = getX + andY = getY + + a <- getA + andB <- getB + + return $ x + andY + a + andB From bd61377e7fed22a742085b7ea29288f979e4a6c9 Mon Sep 17 00:00:00 2001 From: Joe Ferris Date: Mon, 12 May 2014 10:51:00 -0400 Subject: [PATCH 024/488] Rule for trailing commas in multi-line lists Without a trailing comma on the last item in a list, adding a new item causes a deletion and two insertion in Git diffs. With a trailing comma on the last item in a list, adding a new item causes only one insertion and no deletions. --- style/README.md | 4 ++++ style/samples/ruby.rb | 10 +++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/style/README.md b/style/README.md index f9e49886..05a178ae 100644 --- a/style/README.md +++ b/style/README.md @@ -159,8 +159,12 @@ Ruby * Use `def self.method`, not `def Class.method` or `class << self`. * Use `def` with parentheses when there are arguments. * Use `each`, not `for`, for iteration. +* Use a trailing comma after each item in a multi-line list, including the last + item. [Example][trailing comma example] * Use heredocs for multi-line strings. +[trailing comma example]: /style/samples/ruby.rb#L45 + ERb --- diff --git a/style/samples/ruby.rb b/style/samples/ruby.rb index fc8b4a84..6e60c314 100644 --- a/style/samples/ruby.rb +++ b/style/samples/ruby.rb @@ -38,10 +38,18 @@ def method_that_returns_a_hash def method_with_large_hash { :one => 'value', - :two => 'value' + :two => 'value', } end + def method_with_large_array + [ + :one, + :two, + :three, + ] + end + def invoke_method_with_arguments_on_multiple_lines some_method( i_am_a_long_variable_name_that_i_will_never_fit_on_one_line_with_others, From 417a97b7f0701031bed28b0ed7301a0e842cd4df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Quenneville?= Date: Tue, 18 Mar 2014 11:36:57 -0400 Subject: [PATCH 025/488] Clarify guideline on the use of boolean operators The use of the term 'boolean expression' left some uncertainty as to when to use which operators. See https://github.com/thoughtbot/whetstone/pull/30#discussion_r10706657 --- style/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/style/README.md b/style/README.md index 05a178ae..845d0ccd 100644 --- a/style/README.md +++ b/style/README.md @@ -149,9 +149,9 @@ Ruby * Prefer `map` over `collect`. * Prefer `select` over `find_all`. * Prefer single quotes for strings. +* Prefer `&&` and `||` over `and` and `or`. * Use `_` for unused block parameters. * Use `%{}` for single-line strings needing interpolation and double-quotes. -* Use `&&` and `||` for Boolean expressions. * Use `{...}` for single-line blocks. Use `do..end` for multi-line blocks. * Use `?` suffix for predicate methods. * Use `CamelCase` for classes and modules, `snake_case` for variables and From 86260947e1dc340cda4e8039f506dd79997f77fe Mon Sep 17 00:00:00 2001 From: Dan Croak Date: Sat, 17 May 2014 19:52:26 -0700 Subject: [PATCH 026/488] Add environment variables before deploying --- protocol/rails/README.md | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/protocol/rails/README.md b/protocol/rails/README.md index c331b97a..e5341c04 100644 --- a/protocol/rails/README.md +++ b/protocol/rails/README.md @@ -73,12 +73,18 @@ Rails work, look in particular for: Deploy ------ -View a list of new commits. View changed files. Deploy to -[Heroku](https://devcenter.heroku.com/articles/quickstart) staging. +View a list of new commits. View changed files. git fetch staging git log staging/master..master git diff --stat staging/master + +If necessary, add new environment variables. + + heroku config:add NEW_VARIABLE=value --remote staging + +Deploy to [Heroku](https://devcenter.heroku.com/articles/quickstart) staging. + git push staging If necessary, run migrations and restart the dynos. @@ -97,6 +103,7 @@ Deploy to production. git fetch production git log production/master..master git diff --stat production/master + heroku config:add NEW_VARIABLE=value --remote production git push production heroku run rake db:migrate --remote production heroku restart --remote production From b9029451d0486e9bbb72507b2c868bbc2abac0b2 Mon Sep 17 00:00:00 2001 From: Gabe Berke-Williams Date: Fri, 9 May 2014 09:49:36 -0400 Subject: [PATCH 027/488] Add contribution guidelines --- CONTRIBUTING.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 00000000..747830e9 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,10 @@ +# Contributing + +We love pull requests. If you have something you want to add or remove, please +open a new pull request. + +## Getting Feedback + +Since these are our guides, we want everyone at thoughtbot to see them. We have +a lot of offices across a lot of timezones, so we leave all PRs open for at +least a week to get feedback from everyone. From 51ce4dbc401d48f09e5f4803979b8f765bda5df9 Mon Sep 17 00:00:00 2001 From: Kyle Fiedler Date: Thu, 22 May 2014 12:28:28 -0400 Subject: [PATCH 028/488] Specify quotes for Scss --- style/README.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/style/README.md b/style/README.md index 845d0ccd..cf7d625e 100644 --- a/style/README.md +++ b/style/README.md @@ -82,6 +82,7 @@ Sass * Use a space between selector and `{`. * Use single quotation marks for attribute selectors and property values. * Use only lowercase, including colors. +* Use single quotes, including imports. * Don't add a unit specification after `0` values, unless required by a mixin. ### Order @@ -339,12 +340,12 @@ Haskell * Break long expressions before operators or keywords. * Break long type signatures between arguments. -* Order imports in three sections, separating each by a blank line: - [standard libraries], third-party libraries, application modules. - Within each section, alphabetize the imports and place qualified +* Order imports in three sections, separating each by a blank line: + [standard libraries], third-party libraries, application modules. + Within each section, alphabetize the imports and place qualified imports last. * Use comma-first style exports, records, and lists. -* Use four-space indentation except the `where` keyword which is +* Use four-space indentation except the `where` keyword which is indented two spaces [Example][where-example]. [standard libraries]: http://www.haskell.org/ghc/docs/latest/html/libraries/index.html From 3005f8b1da9cf6bf68f71545a2782c25a752e8fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Quenneville?= Date: Thu, 22 May 2014 10:47:42 -0400 Subject: [PATCH 029/488] Use RSpec's `allow` syntax instead of `stub` `allow` was added to RSpec 2 along with `expect` as part of the move away from `should` and `stub` which monkey-patched `Object`. It will be the default in RSpec 3. --- style/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/style/README.md b/style/README.md index cf7d625e..fd9386f3 100644 --- a/style/README.md +++ b/style/README.md @@ -241,11 +241,13 @@ Testing * Prefer `eq` to `==` in RSpec. * Separate setup, exercise, verification, and teardown phases with newlines. * Use RSpec's [`expect` syntax]. +* Use RSpec's [`allow` syntax] for method stubs. * Use `should` shorthand for [one-liners with an implicit subject]. * Use `not_to` instead of `to_not` in RSpec expectations. * Prefer the `have_css` matcher to the `have_selector` matcher in Capybara assertions. [`expect` syntax]: http://myronmars.to/n/dev-blog/2012/06/rspecs-new-expectation-syntax +[`allow` syntax]: https://github.com/rspec/rspec-mocks#method-stubs [one-liners with an implicit subject]: https://github.com/rspec/rspec-expectations/blob/master/Should.md#one-liners #### Acceptance Tests From 82e5eddb0b3b1717852cc8480f0b5e03ac650fed Mon Sep 17 00:00:00 2001 From: Joe Ferris Date: Wed, 4 Jun 2014 15:37:35 -0400 Subject: [PATCH 030/488] Suggest a rebase/squash before pull requests Performing a rebase/squash before a pull request performs an opportunity to craft well-written commits before reviews are performed. This makes the review easier, as the commits are easier to understand, and it allows review of the actual commit titles and messages. --- protocol/git/README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/protocol/git/README.md b/protocol/git/README.md index eb95333e..d3eb651a 100644 --- a/protocol/git/README.md +++ b/protocol/git/README.md @@ -49,6 +49,11 @@ 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: + + git rebase -i origin/master + Share your branch. git push origin From dc293558279e3cc0d8365355334b334b477a7728 Mon Sep 17 00:00:00 2001 From: halogenandtoast Date: Thu, 5 Jun 2014 09:51:48 -0400 Subject: [PATCH 031/488] Remove guide for using single mailer This is not a guideline we often follow. In addition, it's fairly restrictive and leads to poor organization of information that could be better expressed with proper naming. If anything this guide violates our other guide for naming things to reveal intent. --- style/README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/style/README.md b/style/README.md index fd9386f3..30da3ded 100644 --- a/style/README.md +++ b/style/README.md @@ -226,7 +226,6 @@ Background Jobs Email ----- -* Use one `ActionMailer` for the app. Name it `Mailer`. * 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]. From a362a9e74c25100001c19efd849928d4bcdb59dc Mon Sep 17 00:00:00 2001 From: Gabe Berke-Williams Date: Sat, 7 Jun 2014 17:58:11 -0400 Subject: [PATCH 032/488] Add a period --- code-review/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code-review/README.md b/code-review/README.md index e9842a6b..dd508ad1 100644 --- a/code-review/README.md +++ b/code-review/README.md @@ -13,7 +13,7 @@ Everyone * Ask for clarification. ("I didn't understand. Can you clarify?") * Avoid selective ownership of code. ("mine", "not mine", "yours") * Avoid using terms that could be seen as referring to personal traits. ("dumb", - "stupid") Assume everyone is attractive, intelligent, and well-meaning. + "stupid"). Assume everyone is attractive, intelligent, and well-meaning. * Be explicit. Remember people don't always understand your intentions online. * Be humble. ("I'm not sure - let's look it up.") * Don't use hyperbole. ("always", "never", "endlessly", "nothing") From 88ac884f68ca3e5823fab82c4f8620efcee849a2 Mon Sep 17 00:00:00 2001 From: "Jessie A. Young" Date: Fri, 6 Jun 2014 15:43:54 -0400 Subject: [PATCH 033/488] Order ActiveRecord associations above validations * Mirror class order in tests --- style/README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/style/README.md b/style/README.md index 30da3ded..cf0daf3f 100644 --- a/style/README.md +++ b/style/README.md @@ -190,6 +190,7 @@ Rails * Name initializers for their gem name. * Order ActiveRecord associations alphabetically by attribute name. * Order ActiveRecord validations alphabetically by attribute name. +* Order ActiveRecord associations above ActiveRecord validations. * Order controller contents: filters, public methods, private methods. * Order i18n translations alphabetically by key name. * Order model contents: constants, macros, public methods, private methods. @@ -235,8 +236,6 @@ Testing ------- * Avoid the `private` keyword in specs. -* Order ActiveRecord association tests alphabetically by attribute name. -* Order ActiveRecord validation tests alphabetically by attribute name. * Prefer `eq` to `==` in RSpec. * Separate setup, exercise, verification, and teardown phases with newlines. * Use RSpec's [`expect` syntax]. @@ -285,6 +284,8 @@ Testing * Use `context` to describe testing preconditions. * Use `describe '#method_name'` to group tests by method-under-test * Use a single, top-level `describe ClassName` block. +* Order validation, association, and method tests in the same order that they + appear in the class. [Imperative mood]: http://en.wikipedia.org/wiki/Imperative_mood From e220e4381a4b5a315c5bd703ac2bd36862721977 Mon Sep 17 00:00:00 2001 From: Reda Lemeden Date: Fri, 23 May 2014 17:44:38 +0200 Subject: [PATCH 034/488] Add space around operands in Sass --- style/README.md | 5 +++++ style/samples/sass.scss | 1 + 2 files changed, 6 insertions(+) diff --git a/style/README.md b/style/README.md index cf0daf3f..93700546 100644 --- a/style/README.md +++ b/style/README.md @@ -84,6 +84,11 @@ Sass * Use only lowercase, including colors. * Use single quotes, including imports. * Don't add a unit specification after `0` values, unless required by a mixin. +* Use space around operands: `$variable * 1.5`, not `$variable*1.5` +* Avoid in-line operations in shorthand declarations (Ex. `padding: $variable +* * 1.5 variable * 2`) +* Use parentheses around individual operations in shorthand declarations: `padding: +* ($variable * 1.5) ($variable * 2)` ### Order * Use alphabetical order for declarations. diff --git a/style/samples/sass.scss b/style/samples/sass.scss index dedf5149..16e70ea2 100644 --- a/style/samples/sass.scss +++ b/style/samples/sass.scss @@ -17,6 +17,7 @@ section.application { .news { attribute: value; + attribute: ($variable * 1.5) 2em; article { From 2fe25dfe5e6e541652ada45d10cc5d9b7e408c05 Mon Sep 17 00:00:00 2001 From: Reda Lemeden Date: Fri, 23 May 2014 17:46:01 +0200 Subject: [PATCH 035/488] Fix --- style/README.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/style/README.md b/style/README.md index 93700546..ca998bcb 100644 --- a/style/README.md +++ b/style/README.md @@ -85,10 +85,8 @@ Sass * Use single quotes, including imports. * Don't add a unit specification after `0` values, unless required by a mixin. * Use space around operands: `$variable * 1.5`, not `$variable*1.5` -* Avoid in-line operations in shorthand declarations (Ex. `padding: $variable -* * 1.5 variable * 2`) -* Use parentheses around individual operations in shorthand declarations: `padding: -* ($variable * 1.5) ($variable * 2)` +* Avoid in-line operations in shorthand declarations (Ex. `padding: $variable * 1.5 variable * 2`) +* Use parentheses around individual operations in shorthand declarations: `padding: ($variable * 1.5) ($variable * 2)` ### Order * Use alphabetical order for declarations. From 2e9733d1cd73243164548673427e79ed3482ee7e Mon Sep 17 00:00:00 2001 From: Mike Burns Date: Tue, 20 May 2014 10:52:49 +0200 Subject: [PATCH 036/488] Prefer double quotes in Ruby This will reduce the number of changes we have to make: when a string transitions into something that needs interpolation, we do not need to change the quotes. This will also bring consistency to our writing: all strings will now have double quotes, regardless of their contents. It also puts us more in line with a large amount of external Ruby code, which tends to use double quotes. --- style/README.md | 2 +- style/samples/ruby.rb | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/style/README.md b/style/README.md index ca998bcb..64944509 100644 --- a/style/README.md +++ b/style/README.md @@ -152,7 +152,7 @@ Ruby * Prefer `inject` over `reduce`. * Prefer `map` over `collect`. * Prefer `select` over `find_all`. -* Prefer single quotes for strings. +* Prefer double quotes for strings. * Prefer `&&` and `||` over `and` and `or`. * Use `_` for unused block parameters. * Use `%{}` for single-line strings needing interpolation and double-quotes. diff --git a/style/samples/ruby.rb b/style/samples/ruby.rb index 6e60c314..52581598 100644 --- a/style/samples/ruby.rb +++ b/style/samples/ruby.rb @@ -1,5 +1,5 @@ class SomeClass - SOME_CONSTANT = 'upper case name' + SOME_CONSTANT = "upper case name" def initialize(attributes) @some_attribute = attributes[:some_attribute] @@ -32,13 +32,13 @@ def method_that_returns_an_array end def method_that_returns_a_hash - { :key => 'value' } + { :key => "value" } end def method_with_large_hash { - :one => 'value', - :two => 'value', + :one => "value", + :two => "value", } end From 36d9272ea9068353207b3ee21cf1936933dafa7e Mon Sep 17 00:00:00 2001 From: David Alexander Date: Tue, 17 Jun 2014 10:12:17 -0400 Subject: [PATCH 037/488] Fix Git protocol link Rebase workflow changed location to inside of protocol/git/README.md --- style/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/style/README.md b/style/README.md index 64944509..cd9a1e7b 100644 --- a/style/README.md +++ b/style/README.md @@ -16,7 +16,7 @@ Git * Squash multiple trivial commits into a single commit. * Write a [good commit message]. -[rebase workflow]: https://github.com/thoughtbot/guides/tree/master/protocol#merge +[rebase workflow]: https://github.com/thoughtbot/guides/tree/master/protocol/git#merge [good commit message]: http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html Formatting From 7f94caad21cf485b380864afefdfa987887cdc8e Mon Sep 17 00:00:00 2001 From: Tony DiPasquale Date: Tue, 13 May 2014 15:53:48 -0400 Subject: [PATCH 038/488] Update Objective-C style guide --- style/README.md | 19 +++++++++++++++---- style/samples/ObjectiveC.m | 35 +++++++++++++++++++---------------- 2 files changed, 34 insertions(+), 20 deletions(-) diff --git a/style/README.md b/style/README.md index cd9a1e7b..7a4dcb52 100644 --- a/style/README.md +++ b/style/README.md @@ -297,13 +297,17 @@ Objective-C [Sample](samples/ObjectiveC.m) -* `#import` linked frameworks in the prefix header (`ProjectName-Prefix.pch`). -* Keep `.xib` files grouped with their associated view class. +* Setup new projects using liftoff and follow provided directory structure. +* Place `#import`s into the prefix header (`ProjectName-Prefix.pch`) only if + used in _many_ files. +* Place `.xib` files under `Resources/Nibs` and their associated view files in + `Classes/Views`. * Order `#import` statements alphabetically. * Order `@class` directives alphabetically. * Order `@property` modifiers: memory management, atomicity, writability. -* Organize classes into `models`, `views`, `controllers`, `categories`, - and `services` directories. +* Leave out `@property` modifiers unless needed, `nonatomic` is the only one + needed in most cases except connecting views with IB in which case `weak` may + also be needed. * Prefer `@class` to `#import` when referring to external classes in a public `@interface`. * Prefer `@property` to declaring instance variables. @@ -320,6 +324,13 @@ Objective-C * Use `NSAssert` in methods that require the presence of certain arguments. * Write methods using the happy path. Indent the exceptional cases. Keep the optimal case in the left-most column. +* Prefer `enumerateObjectsUsingBlock:` when looping through arrays. +* Always use braces with control and loop blocks unless it can easily fit on + one line. +* Place opening brace for control and loop blocks on same line. +* Prefer `NSInteger`, `CGFloat`, and similar macros over `int`, `float`, and + other base types. +* Prefer AutoLayout for view layouts and constraints. Python ------ diff --git a/style/samples/ObjectiveC.m b/style/samples/ObjectiveC.m index 439f62a3..bcf3d98d 100644 --- a/style/samples/ObjectiveC.m +++ b/style/samples/ObjectiveC.m @@ -2,21 +2,22 @@ #import "Beta.h" // Use @interface extensions for private properties -@interface ClassName () +@interface TBClassName () // Keep @properties grouped together by function -@property (strong, nonatomic) IBOutlet UISearchBar *searchBar; -@property (strong, nonatomic) IBOutlet UITableView *tableView; +@property (nonatomic, weak) IBOutlet UISearchBar *searchBar; +@property (nonatomic, weak) IBOutlet UITableView *tableView; -@property (strong, nonatomic) NSManagedObjectContext *managedObjectContext; -@property (strong, nonatomic) NSFetchedResultsController *fetchedResultsController; +@property (nonatomic) NSManagedObjectContext *managedObjectContext; +@property (nonatomic) NSFetchedResultsController *fetchedResultsController; -@property (strong, nonatomic, readonly) TBObject *someObject; +@property (nonatomic, readonly) TBObject *someObject; @end // Use static NSString points to consts for string constants -static NSString *const ConstantName = @"Constant"; +static NSString *const TBConstantName = @"Constant"; +static NSUInteger const TBNumberOfCardsInDeck = 52; // Prepend constants with 'k' when being used as keys static NSString *const kFirstName = @"FirstName"; @@ -37,8 +38,7 @@ - (id)initWithCoder:(NSCoder *)aDecoder // Return early if conditions prohibit the intended function of the method // Use conditionals for exceptional cases // Keep the 'optimal' path non-indented - if (!self) - return nil; + if (!self) return nil; return self; } @@ -52,18 +52,21 @@ - (void)shuffleCards NSDictionary *themeColors = @{ kRedColor : [UIColor redColor], kBlueColor : [UIColor blueColor] }; NSArray *robots = @[ @"Ralph", @"Bender", @"The Iron Giant" ]; - NSMutableArray *deckOfCards = [NSMutableArray arrayWithCapacity:52]; - - // Newlines before and after conditional blocks - for (Card *card in deckOfCards) - NSLog(@"%@", [card description]); + NSMutableArray *deckOfCards = [NSMutableArray array]; Card *jokerCard = [Card joker]; [deckOfCards addObject:jokerCard]; + // Newlines before and after conditional blocks + // Use enumerate to loop through arrays + // Use explicit class in block parameters + // Use full word 'index' + [deckOfCards enumerateObjectsUsingBlock:^(Card *card, NSUInteger index, BOOL *stop) { + NSLog(@"%@", [card description]); + }]; + // Use ! to check for nots. Comparing to 'nil' is redundant - if (![creditCard isValid]) - { + if (![creditCard isValid]) { //... } } From 072860add2bbd270fddda02b9443485632a5032d Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Fri, 20 Jun 2014 12:00:22 -0400 Subject: [PATCH 039/488] Prefer `!` over `not` --- style/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/style/README.md b/style/README.md index 7a4dcb52..512cedc1 100644 --- a/style/README.md +++ b/style/README.md @@ -132,6 +132,7 @@ CoffeeScript private variables and functions. * Prefer `==` and `!=` to `is` and `isnt` * Prefer `||` and `&&` to `or` and `and` +* Prefer `!` over `not` Ruby ---- @@ -154,6 +155,7 @@ Ruby * Prefer `select` over `find_all`. * Prefer double quotes for strings. * Prefer `&&` and `||` over `and` and `or`. +* Prefer `!` over `not`. * Use `_` for unused block parameters. * Use `%{}` for single-line strings needing interpolation and double-quotes. * Use `{...}` for single-line blocks. Use `do..end` for multi-line blocks. From f682c4bd46c03cf081f6ea96482eda4fa1f492e3 Mon Sep 17 00:00:00 2001 From: Frank Ashurst Date: Mon, 5 May 2014 05:26:57 -0300 Subject: [PATCH 040/488] Update README.md Corrects link to the indentation example on the Haskell section. --- style/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/style/README.md b/style/README.md index 512cedc1..9f4ed75a 100644 --- a/style/README.md +++ b/style/README.md @@ -364,7 +364,7 @@ Haskell imports last. * Use comma-first style exports, records, and lists. * Use four-space indentation except the `where` keyword which is - indented two spaces [Example][where-example]. + indented two spaces. [Example]. [standard libraries]: http://www.haskell.org/ghc/docs/latest/html/libraries/index.html -[where-example] https://github.com/thoughtbot/guides/blob/master/style/samples/haskell.hs#L46 +[Example]: https://github.com/thoughtbot/guides/blob/master/style/samples/haskell.hs#L41 From 22133ddebf6f3c9390453f659b6b2522ef001fa6 Mon Sep 17 00:00:00 2001 From: Joe Ferris Date: Mon, 23 Jun 2014 11:06:53 -0400 Subject: [PATCH 041/488] Add guideline on symbol to_proc * Using &:method_name is brief but clear * Using it consistently aids readability * Preferring simple method calls encourages simple methods --- style/README.md | 2 ++ style/samples/ruby.rb | 8 ++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/style/README.md b/style/README.md index 9f4ed75a..5507b501 100644 --- a/style/README.md +++ b/style/README.md @@ -156,6 +156,8 @@ Ruby * Prefer double quotes for strings. * Prefer `&&` and `||` over `and` and `or`. * Prefer `!` over `not`. +* Prefer `&:method_name` to `{ |item| item.method_name }` for simple method + calls. * Use `_` for unused block parameters. * Use `%{}` for single-line strings needing interpolation and double-quotes. * Use `{...}` for single-line blocks. Use `do..end` for multi-line blocks. diff --git a/style/samples/ruby.rb b/style/samples/ruby.rb index 52581598..682fda67 100644 --- a/style/samples/ruby.rb +++ b/style/samples/ruby.rb @@ -23,8 +23,12 @@ def method_with_multiline_block some_method_after_block(should_follow_after_newline) end - def method_with_single_line_block - items.map { |item| item.some_attribute } + def method_with_single_method_block + items.map(&:some_attribute) + end + + def method_with_oneline_combined_methods_block + items.map { |item| "#{item.one} #{item.two}" } end def method_that_returns_an_array From b9497d2084f24b3b5e88e6e69be82e8b9bc56141 Mon Sep 17 00:00:00 2001 From: George Brocklehurst Date: Mon, 16 Jun 2014 14:17:38 +0200 Subject: [PATCH 042/488] Avoid `link_to` for non-GET requests. * Using `link_to` with the `method` option introduces an un-necessary dependency on JavaScript. * HTML elements -- in this case a button or a link -- should be chosen for their semantics, not their appearance. * CSS can be used to make links look like buttons and buttons look like links. --- style/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/style/README.md b/style/README.md index 5507b501..07da751e 100644 --- a/style/README.md +++ b/style/README.md @@ -204,6 +204,7 @@ Rails * Put application-wide partials in the [`app/views/application`] directory. * Use `def self.method`, not the `scope :method` DSL. * Use the default `render 'partial'` syntax over `render partial: 'partial'`. +* Use `link_to` for GET requests, and `button_to` for other HTTP verbs. [`app/views/application`]: http://asciicasts.com/episodes/269-template-inheritance From abbbe84a3a888effeadcd889a724ed2fdc34a58d Mon Sep 17 00:00:00 2001 From: Brenda Storer Date: Fri, 6 Jun 2014 11:12:47 -0400 Subject: [PATCH 043/488] Add js- prefix to classes as well as ids that use javascript --- style/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/style/README.md b/style/README.md index 07da751e..54f8ea91 100644 --- a/style/README.md +++ b/style/README.md @@ -100,7 +100,7 @@ Sass * Don't use ID's for style. * Use meaningful names: `$visual-grid-color` not `$color` or `$vslgrd-clr`. * Use ID and class names that are as short as possible but as long as necessary. -* Append the prefix js- to ID's that are used by Javascript. +* Prepend the prefix js- to IDs and classes that are used by Javascript. * Avoid using the direct descendant selector `>`. * Avoid nesting more than 4 selectors deep. * Don't nest more than 6 selectors deep. From a6a281e7fb3488fb539bad857953bc50034f1e40 Mon Sep 17 00:00:00 2001 From: Brenda Storer Date: Mon, 23 Jun 2014 11:31:44 -0400 Subject: [PATCH 044/488] Remove guideline to prepend js- to ids using javascript --- style/README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/style/README.md b/style/README.md index 54f8ea91..763a540d 100644 --- a/style/README.md +++ b/style/README.md @@ -100,7 +100,6 @@ Sass * Don't use ID's for style. * Use meaningful names: `$visual-grid-color` not `$color` or `$vslgrd-clr`. * Use ID and class names that are as short as possible but as long as necessary. -* Prepend the prefix js- to IDs and classes that are used by Javascript. * Avoid using the direct descendant selector `>`. * Avoid nesting more than 4 selectors deep. * Don't nest more than 6 selectors deep. From 81cf0e1450ef73b804600857e3d9fb183ac01ec5 Mon Sep 17 00:00:00 2001 From: Dan Croak Date: Sat, 21 Jun 2014 09:17:52 -0700 Subject: [PATCH 045/488] Link to Liftoff Move Liftoff guideline out of style, which is intended to be mostly formatting, and into best practices. --- best-practices/README.md | 2 ++ style/README.md | 3 +-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/best-practices/README.md b/best-practices/README.md index c194a502..e230838a 100644 --- a/best-practices/README.md +++ b/best-practices/README.md @@ -197,6 +197,8 @@ Browsers Objective-C ----------- +* Setup new projects using [Liftoff](https://github.com/thoughtbot/liftoff) and + follow provided directory structure. * Prefer categories on `Foundation` classes to helper methods. * Prefer string constants to literals when providing keys or key paths to methods. diff --git a/style/README.md b/style/README.md index 763a540d..08924ba3 100644 --- a/style/README.md +++ b/style/README.md @@ -301,7 +301,6 @@ Objective-C [Sample](samples/ObjectiveC.m) -* Setup new projects using liftoff and follow provided directory structure. * Place `#import`s into the prefix header (`ProjectName-Prefix.pch`) only if used in _many_ files. * Place `.xib` files under `Resources/Nibs` and their associated view files in @@ -334,7 +333,7 @@ Objective-C * Place opening brace for control and loop blocks on same line. * Prefer `NSInteger`, `CGFloat`, and similar macros over `int`, `float`, and other base types. -* Prefer AutoLayout for view layouts and constraints. +* Prefer *Auto Layout* for view layouts and constraints. Python ------ From ef7b36a280787709ee56910b7e93bf3d48a0ccfa Mon Sep 17 00:00:00 2001 From: Jason Ramirez Date: Fri, 6 Jun 2014 11:28:12 -0400 Subject: [PATCH 046/488] Coffeescript preferred styles * Use `@` instead of `this` for scoping consistency * Prefer double quotes for consistency --- style/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/style/README.md b/style/README.md index 08924ba3..4da4e20f 100644 --- a/style/README.md +++ b/style/README.md @@ -132,6 +132,8 @@ CoffeeScript * Prefer `==` and `!=` to `is` and `isnt` * Prefer `||` and `&&` to `or` and `and` * Prefer `!` over `not` +* Prefer `@` over `this` for referencing instance properties. +* Prefer double quotes. Ruby ---- From 945b32689da821ad16eff8b4ebe94873b9972899 Mon Sep 17 00:00:00 2001 From: Mohnish Jadwani Date: Sat, 21 Jun 2014 00:02:19 +0530 Subject: [PATCH 047/488] Clarify SRP guideline The single responsibility principle sounds more appropriate when referring to a class as a whole rather than an object. --- best-practices/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/best-practices/README.md b/best-practices/README.md index e230838a..81f2cb0d 100644 --- a/best-practices/README.md +++ b/best-practices/README.md @@ -24,8 +24,8 @@ Object-Oriented Design * Limit an object's dependencies (entities that depend on an object). * Prefer composition over inheritance. * Prefer small methods. Between one and five lines is best. -* Prefer small objects with a single, well-defined responsibility. When an - object exceeds 100 lines, it may be doing too many things. +* Prefer small classes with a single, well-defined responsibility. When a + class exceeds 100 lines, it may be doing too many things. * [Tell, don't ask]. [Tell, don't ask]: http://robots.thoughtbot.com/post/27572137956/tell-dont-ask From 98eede4e2d93d76b0e9e5bdedb6899985de6d574 Mon Sep 17 00:00:00 2001 From: Joe Ferris Date: Tue, 1 Jul 2014 10:18:22 -0400 Subject: [PATCH 048/488] Don't use spaces after unary operators The current guide suggests spaces around all operators, but we typically don't add spaces after unary operators, such as `!`. Adopted from #163. --- style/README.md | 5 +++-- style/samples/ruby.rb | 6 +++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/style/README.md b/style/README.md index 4da4e20f..d4e1d8ae 100644 --- a/style/README.md +++ b/style/README.md @@ -40,8 +40,9 @@ Formatting * Use 2 space indentation (no tabs). * Use an empty line between methods. * Use empty lines around multi-line blocks. -* Use spaces around operators, after commas, after colons and semicolons, around - `{` and before `}`. +* Use spaces around operators, except for unary operators, such as `!`. +* Use spaces after commas, after colons and semicolons, around `{` and before + `}`. * Use [Unix-style line endings] (`\n`). * Use [uppercase for SQL key words and lowercase for SQL identifiers]. diff --git a/style/samples/ruby.rb b/style/samples/ruby.rb index 682fda67..771d044a 100644 --- a/style/samples/ruby.rb +++ b/style/samples/ruby.rb @@ -66,10 +66,14 @@ def invoke_method_with_arguments_on_multiple_lines two) end - def method_that_uses_infix_operators + def method_which_uses_infix_operators left + middle - right end + def method_which_uses_unary_operator + !signed_in? + end + def method_without_arguments if complex_condition? positive_branch From 4ba47b5313f0bd5e271ab45bc7169b45120c07a9 Mon Sep 17 00:00:00 2001 From: Gabe Berke-Williams Date: Wed, 7 May 2014 11:30:18 -0400 Subject: [PATCH 049/488] Prefer `cookies.signed` in Rails `cookies` is fine for storing non-sensitive data, but using `cookies.signed` makes sense as a default. Using `cookies.signed` means you don't need to worry about whether data is sensitive. Cookies, even signed ones, can be easily read by the end-user (see http://www.andylindeman.com/2013/02/18/decoding-rails-session-cookies.html). Using `cookies.signed` only prevents the end-user from *writing* them. --- best-practices/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/best-practices/README.md b/best-practices/README.md index 81f2cb0d..9429de34 100644 --- a/best-practices/README.md +++ b/best-practices/README.md @@ -87,10 +87,12 @@ Rails (`user_id`). * Use `db/seeds.rb` for data that is required in all environments. * Use `dev:prime` rake task for development environment seed data. +* Prefer `cookies.signed` over `cookies` to [prevent tampering]. [`.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 Testing ------- From bae91a21a2d2b9ad1ecf8c2715ffe72e033b78db Mon Sep 17 00:00:00 2001 From: Dan Croak Date: Thu, 29 May 2014 12:16:04 -0700 Subject: [PATCH 050/488] Use v#.#.# convention for OSS release commits This keeps us in line with tools such as https://github.com/tpope/git-bump and lets us use tools to auto-promote releases when we have a consistent interface. --- protocol/open-source/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocol/open-source/README.md b/protocol/open-source/README.md index d61fbe2c..3c1e49b4 100644 --- a/protocol/open-source/README.md +++ b/protocol/open-source/README.md @@ -10,6 +10,6 @@ Releasing a Ruby Gem * Run `bundle install` to update `Gemfile.lock`. * Run the test suite. * Edit `NEWS`, `Changelog`, or `README` files if relevant. -* Commit changes like "Bump to 2.1.0". +* Commit changes. Use the convention "v2.1.0" in your commit message. * Run `rake release`, which tags the release, pushes the tag to GitHub, and pushes the gem to Rubygems.org. From 8ddc91bc546f7585736e654c428700d795f645e3 Mon Sep 17 00:00:00 2001 From: Mike Burns Date: Wed, 7 May 2014 09:24:10 +0200 Subject: [PATCH 051/488] Raise instead of returning false in AR callbacks We never check whether a `#destroy` fails in our controllers: def destroy article = Article.find(params[:id]) article.destroy redirect_to articles_url end Because of this history, it would cause confusion and potential bugs if we introduced a normal `before_destroy` into any of our models. Not only will we need to check the result of `#destroy` for any controller using that model, but we'd also need to check the return value for any controller using models that associate with that model. Since callbacks are used for validated database objects and are expected to succeed, we should raise on failure. This will roll back the transaction and notify the exception handler (e.g. Airbrake). This is true of all callbacks, not just `before_destroy`. --- best-practices/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/best-practices/README.md b/best-practices/README.md index 9429de34..c8c6a0a7 100644 --- a/best-practices/README.md +++ b/best-practices/README.md @@ -69,6 +69,8 @@ Rails * Don't change a migration after it has been merged into master if the desired change can be solved with another migration. * Don't reference a model class directly from a view. +* Don't return false from `ActiveModel` callbacks, but instead raise an + exception. * Don't use instance variables in partials. Pass local variables to partials from view templates. * Don't use SQL or SQL fragments (`where('inviter_id IS NOT NULL')`) outside of From 2a2f5f2e68d863a64d2a7064b5a1d19ac774b8db Mon Sep 17 00:00:00 2001 From: Andrew Chen Date: Wed, 9 Jul 2014 17:27:06 +0800 Subject: [PATCH 052/488] Fix link for "Bundler binstubs" --- best-practices/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/best-practices/README.md b/best-practices/README.md index c8c6a0a7..063e8546 100644 --- a/best-practices/README.md +++ b/best-practices/README.md @@ -42,7 +42,7 @@ Ruby * Prefer `private` when indicating scope. Use `protected` only with comparison methods like `def ==(other)`, `def <(other)`, and `def >(other)`. -[Bundler binstubs](https://github.com/sstephenson/rbenv/wiki/Understanding-binstubs) +[Bundler binstubs]: https://github.com/sstephenson/rbenv/wiki/Understanding-binstubs Ruby Gems --------- From bbc29d545080cd7bf0f51919753d02665f3e855f Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Mon, 9 Jun 2014 08:43:48 -0600 Subject: [PATCH 053/488] Prefer `protected` over `private` for non-public attributes - With warnings enabled, private attributes cause a warning on MRI (doubly important for open source libaries, which should not emit warnings) - Reduces churn when adding comparison or equality methods, since you'll likely need to access the attributes of the other object - Reduces churn when adding a writer, since it is impossible to call private writer methods. --- style/README.md | 2 ++ style/samples/ruby.rb | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/style/README.md b/style/README.md index d4e1d8ae..ee325296 100644 --- a/style/README.md +++ b/style/README.md @@ -172,6 +172,8 @@ Ruby * Use a trailing comma after each item in a multi-line list, including the last item. [Example][trailing comma example] * Use heredocs for multi-line strings. +* Prefer `protected` over `private` for non-public `attr_reader`s, `attr_writer`s, + and `attr_accessor`s. [trailing comma example]: /style/samples/ruby.rb#L45 diff --git a/style/samples/ruby.rb b/style/samples/ruby.rb index 771d044a..51628a16 100644 --- a/style/samples/ruby.rb +++ b/style/samples/ruby.rb @@ -93,6 +93,12 @@ def self.class_method method_body end + protected + + attr_reader :foo + attr_accessor :bar + attr_writer :baz + private def complex_condition? From 92510788aa06ebde80c348e5f2a9036cbf4a5d4a Mon Sep 17 00:00:00 2001 From: Greg Lazarev Date: Thu, 3 Jul 2014 17:04:17 -0700 Subject: [PATCH 054/488] Specify guideline for required keyword arguments --- style/README.md | 2 ++ style/samples/ruby.rb | 3 +++ 2 files changed, 5 insertions(+) diff --git a/style/README.md b/style/README.md index ee325296..ad0d2519 100644 --- a/style/README.md +++ b/style/README.md @@ -168,6 +168,7 @@ Ruby methods, `SCREAMING_SNAKE_CASE` for constants. * Use `def self.method`, not `def Class.method` or `class << self`. * Use `def` with parentheses when there are arguments. +* Don't use spaces after required keyword arguments. [Example][required kwargs] * Use `each`, not `for`, for iteration. * Use a trailing comma after each item in a multi-line list, including the last item. [Example][trailing comma example] @@ -176,6 +177,7 @@ Ruby and `attr_accessor`s. [trailing comma example]: /style/samples/ruby.rb#L45 +[required kwargs]: /style/samples/ruby.rb#L16 ERb --- diff --git a/style/samples/ruby.rb b/style/samples/ruby.rb index 51628a16..c041d19c 100644 --- a/style/samples/ruby.rb +++ b/style/samples/ruby.rb @@ -13,6 +13,9 @@ def method_with_arguments(argument_one, argument_two) each_method_lives_on_its_own_line end + def method_with_required_keyword_arguments(one:, two:) + end + def method_with_multiline_block some_method_before_block(should_be_followed_by_a_newline) From 05239f67fa3fbeaae3bf9f79ae08b0b7663d2f3b Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Thu, 26 Jun 2014 10:06:11 -0400 Subject: [PATCH 055/488] Prefer `{Date,Time}.current` over `.today`/`.now` `Date.current` and `Time.current` take `Time.zone` and `config.time_zone` into account, falling back to `Date.today` and `Time.now` respectively if a zone isn't set. --- best-practices/README.md | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/best-practices/README.md b/best-practices/README.md index 063e8546..2333767f 100644 --- a/best-practices/README.md +++ b/best-practices/README.md @@ -90,6 +90,9 @@ Rails * Use `db/seeds.rb` for data that is required in all environments. * Use `dev:prime` rake task for development environment seed data. * Prefer `cookies.signed` over `cookies` to [prevent tampering]. +* Prefer `Time.current` over `Time.now` +* Prefer `Date.current` over `Date.today` +* Prefer `Time.zone.parse("2014-07-04 16:05:37")` over `Time.parse("2014-07-04 16:05:37")` [`.ruby-version`]: https://gist.github.com/fnichol/1912050 [redirects]: http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.30 @@ -209,15 +212,15 @@ Objective-C Shell ----- -* Don't parse the output of `ls`. See [here][parsingls] for details and +* Don't parse the output of `ls`. See [here][parsingls] for details and alternatives. -* Don't use `cat` to provide a file on `stdin` to a process that accepts +* Don't use `cat` to provide a file on `stdin` to a process that accepts file arguments itself. -* Don't use a `/bin/sh` [shebang][] unless you plan to test and run your - script on at least: Actual Sh, Dash in POSIX-compatible mode (as it - will be run on Debian), and Bash in POSIX-compatible mode (as it will +* Don't use a `/bin/sh` [shebang][] unless you plan to test and run your + script on at least: Actual Sh, Dash in POSIX-compatible mode (as it + will be run on Debian), and Bash in POSIX-compatible mode (as it will be run on OSX). -* Don't use any non-POSIX [features][bashisms] when using a `/bin/sh` +* Don't use any non-POSIX [features][bashisms] when using a `/bin/sh` [shebang][]. * If calling `cd`, have code to handle a failure to change directories. * If calling `rm` with a variable, ensure the variable is not empty. @@ -230,16 +233,16 @@ Shell * Prefer `printf` over `echo`. * Prefer `sed '/re/!d; s//.../'` to `grep re | sed 's/re/.../'`. * Prefer `sed 'cmd; cmd'` to `sed -e 'cmd' -e 'cmd'`. -* Prefer checking exit statuses over output in `if` statements (`if grep +* Prefer checking exit statuses over output in `if` statements (`if grep -q ...; `, not `if [ -n "$(grep ...)" ];`). -* Prefer reading environment variables over process output (`$TTY` not +* Prefer reading environment variables over process output (`$TTY` not `$(tty)`, `$PWD` not `$(pwd)`, etc). * Use `$( ... )`, not backticks for capturing command output. * Use `$(( ... ))`, not `expr` for executing arithmetic expressions. -* Use `1` and `0`, not `true` and `false` to represent boolean +* Use `1` and `0`, not `true` and `false` to represent boolean variables. * Use `find -print0 | xargs -0`, not `find | xargs`. -* Use quotes around every `"$variable"` and `"$( ... )"` expression +* Use quotes around every `"$variable"` and `"$( ... )"` expression unless you want them to be word-split and/or interpreted as globs. * Use the `local` keyword with function-scoped variables. * Identify common problems with [shellcheck][]. From 7573ec405d989d09fb5a9b7796fdb6c5dbbfa765 Mon Sep 17 00:00:00 2001 From: Reda Lemeden Date: Fri, 18 Jul 2014 15:36:03 +0000 Subject: [PATCH 056/488] Use double quotes in Sass --- style/README.md | 3 +-- style/samples/sass.scss | 7 ++++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/style/README.md b/style/README.md index ad0d2519..3ff1490c 100644 --- a/style/README.md +++ b/style/README.md @@ -81,9 +81,8 @@ Sass * Prefer hex color codes `#000`. * Use `//` for comment blocks not `/* */`. * Use a space between selector and `{`. -* Use single quotation marks for attribute selectors and property values. +* Use double quotation marks. * Use only lowercase, including colors. -* Use single quotes, including imports. * Don't add a unit specification after `0` values, unless required by a mixin. * Use space around operands: `$variable * 1.5`, not `$variable*1.5` * Avoid in-line operations in shorthand declarations (Ex. `padding: $variable * 1.5 variable * 2`) diff --git a/style/samples/sass.scss b/style/samples/sass.scss index 16e70ea2..58392f37 100644 --- a/style/samples/sass.scss +++ b/style/samples/sass.scss @@ -1,3 +1,5 @@ +@import "bourbon"; + section.application { @extend %extend; @include mixin; @@ -15,12 +17,15 @@ section.application { attribute: value; } + &:before { + content: ">"; + } + .news { attribute: value; attribute: ($variable * 1.5) 2em; article { - @extend media(value) { attribute: value; } From 52bbab85ea216ad9b1b191d68a319ec97c6bdb3d Mon Sep 17 00:00:00 2001 From: Joe Ferris Date: Mon, 21 Jul 2014 11:47:59 -0400 Subject: [PATCH 057/488] Add note about allowing for feedback * We've had several pull requests merged quickly * Our contribution guidelines request a week to allow feedback --- README.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/README.md b/README.md index 68b46769..1fa5c69d 100644 --- a/README.md +++ b/README.md @@ -26,6 +26,16 @@ A note on the language: * "Prefer" indicates a better option and its alternative to watch out for. * "Use" is a positive instruction. +Contributing +------------ + +Please read the [contribution guidelines] before submitting a pull request. + +In particular: **if you have commit access, please don't merge changes without +waiting a week for everybody to leave feedback**. + +[contribution guidelines]: /CONTRIBUTING.md + Credits ------- From 3bffa96a50fc8b0fc07e471280cb356ca6bb632e Mon Sep 17 00:00:00 2001 From: Dan Croak Date: Tue, 24 Jun 2014 21:54:53 -0700 Subject: [PATCH 058/488] Add initial Backbone guidelines Conventions in the Backbone.js community: http://ricostacruz.com/backbone-patterns/#conventions https://learn.thoughtbot.com/products/1-backbone-js-on-rails https://learn.thoughtbot.com/workshops/22-hands-on-backbone-js-on-rails The common alternative naming convention `App.Models.Photo` / `App.Collections.Photos` / `App.Views.Photo` is unnecessary in Asynchronous Module Definition-style projects using a library like RequireJS. http://requirejs.org/docs/whyamd.html --- style/README.md | 16 ++++++++++++++++ style/samples/backbone.coffee | 20 ++++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 style/samples/backbone.coffee diff --git a/style/README.md b/style/README.md index 3ff1490c..bcb5325a 100644 --- a/style/README.md +++ b/style/README.md @@ -73,7 +73,9 @@ Organization Sass ---- + ### Formatting + * Use the *Scss* syntax. * Use hyphens when naming mixins, extends, classes, functions & variables: `span-columns` not `span_columns` or `spanColumns`. * Use space between property and value: `width: 20px` not `width:20px`. @@ -89,6 +91,7 @@ Sass * Use parentheses around individual operations in shorthand declarations: `padding: ($variable * 1.5) ($variable * 2)` ### Order + * Use alphabetical order for declarations. * Place @extends and @includes at the top of your declaration list. * Place media queries directly after the declaration list. @@ -97,6 +100,7 @@ Sass * Place nested selectors last. ### Selectors + * Don't use ID's for style. * Use meaningful names: `$visual-grid-color` not `$color` or `$vslgrd-clr`. * Use ID and class names that are as short as possible but as long as necessary. @@ -111,6 +115,7 @@ Sass * Avoid nesting within a media query. ### Organization + * Use Bourbon for a Sass Library. * Use Neat for a grid framework. * Use Bitters / Base folder for style on HTML tags, global variables, global extends and global mixins. @@ -135,6 +140,17 @@ CoffeeScript * Prefer `@` over `this` for referencing instance properties. * Prefer double quotes. +Backbone +-------- + +[Sample](samples/backbone.coffee) + +* Organize all objects in one `window.App` namespace. +* Name collections the plural version of the model. +* Name models without a suffix. +* Name the router `App.Router`. +* Name views with a `View` suffix. + Ruby ---- diff --git a/style/samples/backbone.coffee b/style/samples/backbone.coffee new file mode 100644 index 00000000..2b089558 --- /dev/null +++ b/style/samples/backbone.coffee @@ -0,0 +1,20 @@ +# app.coffee +window.App = + initialize: -> + App.router = new App.Router() + Backbone.history.start(pushState: true) + +$(document).ready -> + App.initialize() + +# collections/photos.coffee +class App.Photos extends Backbone.Collection + +# models/photo.coffee +class App.Photo extends Backbone.Model + +# router.coffee +class App.Router extends Backbone.Router + +# views/photo.coffee +class App.PhotoView extends Backbone.View From e69891a99155f15a61f294162b8e8b5b0def0fd6 Mon Sep 17 00:00:00 2001 From: Dan Croak Date: Wed, 16 Jul 2014 12:22:31 -0700 Subject: [PATCH 059/488] Add "Accepting a GitHub Pull Request" protocol The only potentially controversial part of this protocol is the `co-pr` alias. In general, we've avoided using aliases and non-standard scripts when describing our protocol. For example, in the [Git Protocol], commands such as `git fetch origin` and `git rebase -i origin/master` are spelled out, even though just about everybody uses an alias for these steps. [Git Protocol]: https://github.com/thoughtbot/guides/tree/master/protocol/git My argument for showing the alias here is that I don't think it's very pragmatic to have the pull request number repeated three times in the guide: git fetch origin pull/123/head:pr/123 git checkout pr/123 As programmers, we're all likely to use an alias for this. I think the right balance is to define the alias in the guide, and then use it. --- protocol/open-source/README.md | 38 ++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/protocol/open-source/README.md b/protocol/open-source/README.md index 3c1e49b4..3432de0d 100644 --- a/protocol/open-source/README.md +++ b/protocol/open-source/README.md @@ -3,6 +3,44 @@ Open Source Protocol A guide for releasing and maintaining open source projects. +Accepting a GitHub Pull Request +------------------------------- + +Given you have this in your `~/.gitconfig`: + + [alias] + co-pr = !sh -c 'git fetch origin pull/$1/head:pr/$1 && git checkout pr/$1' - + +Check out the code by its GitHub pull request number: + + git co-pr 123 + +Rebase interactively, squash, and potentially improve commit messages: + + git rebase -i master + +Look at changes: + + git diff origin/master + +Run the code and tests. For example, on a Ruby project: + + bundle + rake + +Merge code into master: + + git checkout master + git merge pr/123 --ff-only + +Push: + + git push origin master + +Clean up: + + git branch -D pr/123 + Releasing a Ruby Gem -------------------- From 6f762272ba611fa43f2814b7e3026e7823c968ce Mon Sep 17 00:00:00 2001 From: Tyson Gach Date: Thu, 24 Jul 2014 13:18:01 -0400 Subject: [PATCH 060/488] Specify decimal number leading zeros in Sass --- style/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/style/README.md b/style/README.md index bcb5325a..e473abc6 100644 --- a/style/README.md +++ b/style/README.md @@ -86,6 +86,7 @@ Sass * Use double quotation marks. * Use only lowercase, including colors. * Don't add a unit specification after `0` values, unless required by a mixin. +* Use a leading zero in decimal numbers: `0.5` not `.5` * Use space around operands: `$variable * 1.5`, not `$variable*1.5` * Avoid in-line operations in shorthand declarations (Ex. `padding: $variable * 1.5 variable * 2`) * Use parentheses around individual operations in shorthand declarations: `padding: ($variable * 1.5) ($variable * 2)` From e336dbe9c8c9c10c64c7f3baed5a5c17c98307d5 Mon Sep 17 00:00:00 2001 From: Robert Eshleman Date: Wed, 6 Aug 2014 14:14:11 -0400 Subject: [PATCH 061/488] Fix broken link in "Rails Migrations" --- style/README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/style/README.md b/style/README.md index e473abc6..9b7d962c 100644 --- a/style/README.md +++ b/style/README.md @@ -234,8 +234,7 @@ Rails Migrations ---------------- * Set an empty string as the default constraint for non-required string and text fields. [Example][default example]. -* List timestamps first when creating a new table. [Example][timestamps - example]. +* List timestamps first when creating a new table. [Example][timestamps example]. [timestamps example]: /style/samples/migration.rb [default example]: /style/samples/migration.rb#L6 From 3df6d142367a421c68c79ed0e99490c07fe7599d Mon Sep 17 00:00:00 2001 From: Harlow Ward Date: Sun, 20 Jul 2014 12:18:53 -0700 Subject: [PATCH 062/488] Add multi-line block to Ruby sample * Update the block to have multiple lines --- style/samples/ruby.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/style/samples/ruby.rb b/style/samples/ruby.rb index c041d19c..d076b963 100644 --- a/style/samples/ruby.rb +++ b/style/samples/ruby.rb @@ -21,6 +21,7 @@ def method_with_multiline_block items.each do |item| do_something_with_item + perform_another_action end some_method_after_block(should_follow_after_newline) From 549681ce468a352301c99419d005774ed8cfea2c Mon Sep 17 00:00:00 2001 From: Pavel Bucka Date: Thu, 7 Aug 2014 22:47:19 +0200 Subject: [PATCH 063/488] Fix link for trailing comma example --- style/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/style/README.md b/style/README.md index 9b7d962c..694dbaf5 100644 --- a/style/README.md +++ b/style/README.md @@ -192,7 +192,7 @@ Ruby * Prefer `protected` over `private` for non-public `attr_reader`s, `attr_writer`s, and `attr_accessor`s. -[trailing comma example]: /style/samples/ruby.rb#L45 +[trailing comma example]: /style/samples/ruby.rb#L49 [required kwargs]: /style/samples/ruby.rb#L16 ERb From 2f19ea600bd92dca338bf9e387144e2112a32858 Mon Sep 17 00:00:00 2001 From: "Jessie A. Young" Date: Tue, 19 Aug 2014 10:43:39 -0700 Subject: [PATCH 064/488] Use relative links --- style/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/style/README.md b/style/README.md index 694dbaf5..9769259e 100644 --- a/style/README.md +++ b/style/README.md @@ -16,7 +16,7 @@ Git * Squash multiple trivial commits into a single commit. * Write a [good commit message]. -[rebase workflow]: https://github.com/thoughtbot/guides/tree/master/protocol/git#merge +[rebase workflow]: /protocol/git#merge [good commit message]: http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html Formatting @@ -46,7 +46,7 @@ Formatting * Use [Unix-style line endings] (`\n`). * Use [uppercase for SQL key words and lowercase for SQL identifiers]. -[dot guideline example]: https://github.com/thoughtbot/guides/blob/master/style/samples/ruby.rb#L11 +[dot guideline example]: /style/samples/ruby.rb#L11 [uppercase for SQL key words and lowercase for SQL identifiers]: http://www.postgresql.org/docs/9.2/static/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS [Unix-style line endings]: http://unix.stackexchange.com/questions/23903/should-i-end-my-text-script-files-with-a-newline @@ -390,4 +390,4 @@ Haskell indented two spaces. [Example]. [standard libraries]: http://www.haskell.org/ghc/docs/latest/html/libraries/index.html -[Example]: https://github.com/thoughtbot/guides/blob/master/style/samples/haskell.hs#L41 +[Example]: /style/samples/haskell.hs#L41 From 340b8fd1097b884eab0dc46b5f718797734ef4a5 Mon Sep 17 00:00:00 2001 From: "Jessie A. Young" Date: Fri, 13 Jun 2014 11:23:05 -0700 Subject: [PATCH 065/488] Avoid checking boolean equality directly * Predicate methods should return true or false, and should be tested as such * Custom RSpec matchers are more readable --- style/README.md | 3 +++ style/samples/predicate_tests.rb | 17 +++++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 style/samples/predicate_tests.rb diff --git a/style/README.md b/style/README.md index 9769259e..2f9dbf1c 100644 --- a/style/README.md +++ b/style/README.md @@ -265,6 +265,8 @@ Testing ------- * Avoid the `private` keyword in specs. +* Avoid checking boolean equality directly. Instead, write predicate methods and + use appropriate matchers. [Example][predicate-example]. * Prefer `eq` to `==` in RSpec. * Separate setup, exercise, verification, and teardown phases with newlines. * Use RSpec's [`expect` syntax]. @@ -276,6 +278,7 @@ Testing [`expect` syntax]: http://myronmars.to/n/dev-blog/2012/06/rspecs-new-expectation-syntax [`allow` syntax]: https://github.com/rspec/rspec-mocks#method-stubs [one-liners with an implicit subject]: https://github.com/rspec/rspec-expectations/blob/master/Should.md#one-liners +[predicate-example]: /style/samples/predicate_tests.rb #### Acceptance Tests diff --git a/style/samples/predicate_tests.rb b/style/samples/predicate_tests.rb new file mode 100644 index 00000000..d4c10e91 --- /dev/null +++ b/style/samples/predicate_tests.rb @@ -0,0 +1,17 @@ +# Class under test: + +class Thing + def awesome? + true + end +end + +# RSpec test: + +describe Thing, "#awesome?" do + it "is true" do + thing = Thing.new + + expect(thing).to be_awesome + end +end From b14ce5299ca33e203861be6abc5ab193f5fba7aa Mon Sep 17 00:00:00 2001 From: Dan Croak Date: Sat, 16 Aug 2014 15:48:28 -0700 Subject: [PATCH 066/488] Re-word to use "suffix" Order guidelines alphabetically. --- style/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/style/README.md b/style/README.md index 2f9dbf1c..8ed6e207 100644 --- a/style/README.md +++ b/style/README.md @@ -56,12 +56,12 @@ Naming * Avoid abbreviations. * Avoid object types in names (`user_array`, `email_method` `CalculatorClass`, `ReportModule`). * Name the enumeration parameter the singular of the collection. +* Name variables created by a factory after the factory (`user_factory` + creates `user`). * Name variables, methods, and classes to reveal intent. * Treat acronyms as words in names (`XmlHttpRequest` not `XMLHTTPRequest`), even if the acronym is the entire name (`class Html` not `class HTML`). -* Name variables holding a factory with `_factory` (`user_factory`). -* Name variables created by a factory after the factory (`user_factory` - creates `user`). +* Suffix variables holding a factory with `_factory` (`user_factory`). Organization ------------ From 024079c0ec3692368f8c77002347a31b3a023567 Mon Sep 17 00:00:00 2001 From: Dan Croak Date: Sat, 16 Aug 2014 16:11:38 -0700 Subject: [PATCH 067/488] Avoid backticks in CoffeeScript Backticks allow snippets of JavaScript to be embedded in CoffeeScript. While some folks consider backticks useful in a few niche circumstances, they should be avoided because so none of JavaScript's "bad parts", like with and eval, sneak into CoffeeScript. This can be enforced by Hound with the `no_backticks` option in CoffeeLint, which is enabled by default. http://www.coffeelint.org/#options This is an "Avoid", not a "Don't", because in `ember-cli` the import statements do not work in CoffeeScript and we have to use the backticks: http://www.ember-cli.com/#coffeescript There are some cases where it is unavoidable. Also: alphabetize the CoffeeScript guidelines. --- style/README.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/style/README.md b/style/README.md index 8ed6e207..51a60b16 100644 --- a/style/README.md +++ b/style/README.md @@ -129,17 +129,18 @@ CoffeeScript ------------ * Avoid conditional modifiers (lines that end with conditionals). +* Avoid backticks. * Initialize arrays using `[]`. * Initialize empty objects and hashes using `{}`. -* Use hyphen-separated filenames, such as `coffee-script.coffee`. -* Use `PascalCase` for classes, `lowerCamelCase` for variables and functions, - `SCREAMING_SNAKE_CASE` for constants, `_single_leading_underscore` for - private variables and functions. * Prefer `==` and `!=` to `is` and `isnt` * Prefer `||` and `&&` to `or` and `and` * Prefer `!` over `not` * Prefer `@` over `this` for referencing instance properties. * Prefer double quotes. +* Use hyphen-separated filenames, such as `coffee-script.coffee`. +* Use `PascalCase` for classes, `lowerCamelCase` for variables and functions, + `SCREAMING_SNAKE_CASE` for constants, `_single_leading_underscore` for + private variables and functions. Backbone -------- From 677fed4806760bdc9527ccf040b36e388c73efc4 Mon Sep 17 00:00:00 2001 From: Dan Croak Date: Sat, 16 Aug 2014 15:54:54 -0700 Subject: [PATCH 068/488] Add CoffeeScript colon assignment spaces guideline This can be enforced in Hound with the `colon_assignment_spacing` option in CoffeeLint. http://www.coffeelint.org/#options --- style/README.md | 4 ++++ style/samples/coffee.coffee | 4 ++++ 2 files changed, 8 insertions(+) create mode 100644 style/samples/coffee.coffee diff --git a/style/README.md b/style/README.md index 51a60b16..e665af79 100644 --- a/style/README.md +++ b/style/README.md @@ -128,6 +128,8 @@ Sass CoffeeScript ------------ +[Sample](samples/coffee.coffee) + * Avoid conditional modifiers (lines that end with conditionals). * Avoid backticks. * Initialize arrays using `[]`. @@ -141,6 +143,8 @@ CoffeeScript * Use `PascalCase` for classes, `lowerCamelCase` for variables and functions, `SCREAMING_SNAKE_CASE` for constants, `_single_leading_underscore` for private variables and functions. +* Use zero spaces before and one space after the colon in a colon assignment + (i.e. classes, objects). Backbone -------- diff --git a/style/samples/coffee.coffee b/style/samples/coffee.coffee new file mode 100644 index 00000000..bcecaea9 --- /dev/null +++ b/style/samples/coffee.coffee @@ -0,0 +1,4 @@ +object = { spacing: true } + +class Cat + canBark: false From 28c8078f1eaff18eee68ecd5015fc92dea3d2e1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?U=C4=A3is=20Ozols?= Date: Sun, 14 Sep 2014 16:46:18 +0200 Subject: [PATCH 069/488] Remove extra : from link. --- protocol/git/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocol/git/README.md b/protocol/git/README.md index d3eb651a..eb8fa61a 100644 --- a/protocol/git/README.md +++ b/protocol/git/README.md @@ -47,7 +47,7 @@ Write a [good commit message]. Example format: * More information about commit (under 72 characters). * More information about commit (under 72 characters). - http:://project.management-system.com/ticket/123 + 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: From accdda63a45c772dd8132ff0869c93c78e11fe17 Mon Sep 17 00:00:00 2001 From: Tony DiPasquale Date: Fri, 20 Jun 2014 16:46:25 -0400 Subject: [PATCH 070/488] Add first pass at Swift style guide --- style/README.md | 12 ++++++++ style/samples/Swift.swift | 65 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) create mode 100644 style/samples/Swift.swift diff --git a/style/README.md b/style/README.md index e665af79..ff0c36d7 100644 --- a/style/README.md +++ b/style/README.md @@ -365,6 +365,18 @@ Objective-C other base types. * Prefer *Auto Layout* for view layouts and constraints. +Swift +----- + +[Sample](samples/Swift.swift) + +* Keep up with the Objective-C style guide above. Will highlight differences + here. +* Use `let` whenever possible to make immutable variables +* Name all parameters in functions and enum cases +* Use trailing closures +* Let the compiler infer the type whenever possible + Python ------ diff --git a/style/samples/Swift.swift b/style/samples/Swift.swift new file mode 100644 index 00000000..f6ae5532 --- /dev/null +++ b/style/samples/Swift.swift @@ -0,0 +1,65 @@ + +import Foundation // or not + +// Closures ----------------------------------------------------------- + +// Use typealias when closures are referenced in multiple places +typealias CoolClosure = (foo: Int) -> Bool + +// Use aliased parameter names when function parameters are ambiguous +func yTown(some: Int, withCallback callback: CoolClosure) -> Bool { + return CoolClosure(some) +} + +// It's OK to use $ variable references if the closer is very short and +// readability is maintained +let cool = yTown(5) { $0 == 6 } + +// Use full variable names when closures are more complex +let cool = yTown(5) { foo in + if foo > 5 && foo < 0 { + return true + } else { + return false + } +} + +// Optionals ----------------------------------------------------------- + +var maybe: Bool? + +// Use if-let syntax to unwrap optionals +if let definitely = maybe { + println("This is \(definitely) here") +} + +// If the API you are using has implicit unwrapping you should still use if-let +func tableView(tableView: UITableView!, cellForRowAtIndexPath indexPath: NSIndexPath!) -> UITableViewCell! { + if let index = indexPath { + if let table = tableView { + let cell = table.dequeue.... + return cell; + } + } + + return .None +} + +// Enums -------------------------------------------------------------- + +enum Response { + case Success(NSData) + case Failure(NSError) +} + +// when the type is known you can let the compiler infer +let response: Response = .Success(NSData()) + +switch response { +case let .Success(data): + println("The response returned successfully \(data)") + +case let .Failure(error): + println("An error occured: \(error)") +} + From fb2e3ce9a127f069b7f2116852c02a5621fb87a1 Mon Sep 17 00:00:00 2001 From: Greg Lazarev Date: Thu, 18 Sep 2014 10:58:38 -0700 Subject: [PATCH 071/488] Add guideline to avoid naming classes after patterns --- style/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/style/README.md b/style/README.md index ff0c36d7..a7b4968c 100644 --- a/style/README.md +++ b/style/README.md @@ -55,6 +55,7 @@ Naming * Avoid abbreviations. * Avoid object types in names (`user_array`, `email_method` `CalculatorClass`, `ReportModule`). +* Avoid including pattern names in class names (`NullAddress`, `ReportStrategy`, `AccountVisitor`). * Name the enumeration parameter the singular of the collection. * Name variables created by a factory after the factory (`user_factory` creates `user`). From 12556d83889afa39d86439a4205dfe18b88ea470 Mon Sep 17 00:00:00 2001 From: Greg Lazarev Date: Fri, 19 Sep 2014 10:31:02 -0700 Subject: [PATCH 072/488] Reword per suggestion --- style/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/style/README.md b/style/README.md index a7b4968c..5093aee6 100644 --- a/style/README.md +++ b/style/README.md @@ -55,7 +55,8 @@ Naming * Avoid abbreviations. * Avoid object types in names (`user_array`, `email_method` `CalculatorClass`, `ReportModule`). -* Avoid including pattern names in class names (`NullAddress`, `ReportStrategy`, `AccountVisitor`). +* Prefer naming classes after domain concepts rather than patterns they + implement (e.g. `Guest` vs `NullUser`, `CachedRequest` vs `RequestDecorator`). * Name the enumeration parameter the singular of the collection. * Name variables created by a factory after the factory (`user_factory` creates `user`). From 100fec2578eae72e388e2d16361ac7c95e617ad0 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Wed, 8 Oct 2014 10:31:14 -0600 Subject: [PATCH 073/488] Add style guide for order of properties in Android views Alphabetized is pretty much the standard, but we felt that `id`, `layout_width`, and `layout_height` should be listed first. `id` is the property you care about most (if it is present), and should be quick to find at a glance. `layout_width` and `layout_height` are required on all views, and affect the layout of the view most, so they should also be separated from the rest. --- style/README.md | 7 +++++++ style/samples/android_layout.xml | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 style/samples/android_layout.xml diff --git a/style/README.md b/style/README.md index 5093aee6..06f76f81 100644 --- a/style/README.md +++ b/style/README.md @@ -411,5 +411,12 @@ Haskell * Use four-space indentation except the `where` keyword which is indented two spaces. [Example]. +Android +------- + +* Properties of views should be alphabetized, with the exception of `id`, + `layout_width`, and `layout_height` which should be placed first in that + order. + [standard libraries]: http://www.haskell.org/ghc/docs/latest/html/libraries/index.html [Example]: /style/samples/haskell.hs#L41 diff --git a/style/samples/android_layout.xml b/style/samples/android_layout.xml new file mode 100644 index 00000000..696d758b --- /dev/null +++ b/style/samples/android_layout.xml @@ -0,0 +1,19 @@ + + + + From be66bf6b7f031f6b25f6c40f7ca2f4191fae9c3c Mon Sep 17 00:00:00 2001 From: Pavel Bucka Date: Wed, 13 Aug 2014 12:14:34 +0200 Subject: [PATCH 074/488] Add missing links to samples --- style/README.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/style/README.md b/style/README.md index 06f76f81..d7b2218e 100644 --- a/style/README.md +++ b/style/README.md @@ -76,6 +76,8 @@ Organization Sass ---- +[Sample](samples/sass.scss) + ### Formatting * Use the *Scss* syntax. @@ -239,6 +241,9 @@ Rails Rails Migrations ---------------- + +[Sample](samples/migration.rb) + * Set an empty string as the default constraint for non-required string and text fields. [Example][default example]. * List timestamps first when creating a new table. [Example][timestamps example]. @@ -401,6 +406,8 @@ Shell Haskell ------- +[Sample](samples/haskell.hs) + * Break long expressions before operators or keywords. * Break long type signatures between arguments. * Order imports in three sections, separating each by a blank line: From acfb27511224ad775d62efa8bb77481535356a25 Mon Sep 17 00:00:00 2001 From: Dan Croak Date: Sun, 21 Sep 2014 19:46:26 -0700 Subject: [PATCH 075/488] Add "Product Review" protocol before "Code Review" Our core workflow has been: * Work in feature branch. * Open GitHub pull request for code review. * Make changes based on teammate feedback. * Merge into master. * Push to staging. * Get feedback on the product in-browser from teammates. This generally works well. However, when something isn't quite right with the product and the developer needs to make changes, the feedback loop is too slow. Fast-moving startup teams may want to blame code reviews for causing problems. However, fewer or sloppier reviews lead to problems like bugs and hard-to-change code. We still highly value code reviews. Our hypothesis is that the root problem is that the steps are out of order. What if we performed "Product Review" before "Code Review"? ![Product before Code](http://f.cl.ly/items/1k2b2T1b3v2O1d0A0p3z/Screen%20Shot%202013-05-19%20at%204.57.03%20PM.png) Moving "Product Review" before "Code Review" in our core workflow can be solved in different ways. The most effective is when [teammates are available in person. However, that's not always feasible. Many teams value remote work due to teammates' choices of where they want to live and when they want to work. In the absence of *always* having teammates who are *always* available in person, we have [experimented with approaches] for about 18 months. [experimented with approaches]: http://botcave.thoughtbot.com/ssh-tunnels-for-sharing-development-branches-in-browser-development-platform-as-a-service-and-more This pull request captures the conclusions of the successful experiments. --- protocol/ios/README.md | 7 ++++- protocol/product-review/README.md | 46 +++++++++++++++++++++++++++++++ protocol/rails/README.md | 9 ++++-- 3 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 protocol/product-review/README.md diff --git a/protocol/ios/README.md b/protocol/ios/README.md index cf91029d..184eab8d 100644 --- a/protocol/ios/README.md +++ b/protocol/ios/README.md @@ -41,7 +41,12 @@ Install the app's dependencies. Git Protocol ------------ -Follow the normal [Git Protocol](/protocol/git). +Follow the normal [Git protocol](/protocol/git). + +Product Review +-------------- + +Follow the normal [Product Review protocol](/protocol/product-review). Code Review ----------- diff --git a/protocol/product-review/README.md b/protocol/product-review/README.md new file mode 100644 index 00000000..43e303eb --- /dev/null +++ b/protocol/product-review/README.md @@ -0,0 +1,46 @@ +Product Review +============== + +Cut down cycle time and focus on the user +by getting a teammate to review your changes to the product +before you get a code review or deploy to staging. + +For each change, choose one of four techniques: + +* In-person +* Screencast +* SSH tunnel +* Video chat and screenshare + +In-person +--------- + +If they are sitting next to you, +have them review the changes in person. + +Screencast +---------- + +Use [Licecap] to share a screencast gif in the project's [Slack] channel. + +[Licecap]: http://www.cockos.com/licecap/ +[Slack]: https://slack.com/ + +SSH tunnel +---------- + +Use [ngrok] to set up an SSH tunnel to your work in progress on your laptop: + +[ngrok]: https://ngrok.com/ + + ngrok -subdomain=feature-branch-name 3000 + +Then, share the ngrok URL in the project's Slack channel. + +Video chat and screenshare +-------------------------- + +Start a Google Hangout in the project's Slack channel: + + /hangout + diff --git a/protocol/rails/README.md b/protocol/rails/README.md index e5341c04..aeae7f97 100644 --- a/protocol/rails/README.md +++ b/protocol/rails/README.md @@ -44,14 +44,19 @@ Use [Foreman](https://github.com/ddollar/foreman) to run the app locally. foreman start -It uses your `.env` file and `Procfile` to run processes just like Heroku's -[Cedar](https://devcenter.heroku.com/articles/cedar/) stack. +It uses your `.env` file and `Procfile` to run processes +like Heroku's [Cedar](https://devcenter.heroku.com/articles/cedar/) stack. Git Protocol ------------ Follow the normal [Git Protocol](/protocol/git). +Product Review +-------------- + +Follow the normal [Product Review protocol](/protocol/product-review). + Code Review ----------- From 083cab7b2208b01165feea99ed1c9915f1eb2b5e Mon Sep 17 00:00:00 2001 From: patrick brisbin Date: Tue, 4 Nov 2014 09:57:54 -0500 Subject: [PATCH 076/488] Rewrite `printf` vs `echo` guideline If not passing options, including escapes, or including variables (which may contain escapes), `echo` will work consistently across systems. It should not be broadly avoided as this simple and safe use-case is common. http://unix.stackexchange.com/questions/65803/why-is-printf-better-than-echo --- best-practices/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/best-practices/README.md b/best-practices/README.md index 2333767f..d41aec92 100644 --- a/best-practices/README.md +++ b/best-practices/README.md @@ -216,6 +216,8 @@ Shell alternatives. * Don't use `cat` to provide a file on `stdin` to a process that accepts file arguments itself. +* Don't use `echo` with options, escapes, or variables (use `printf` for those + cases). * Don't use a `/bin/sh` [shebang][] unless you plan to test and run your script on at least: Actual Sh, Dash in POSIX-compatible mode (as it will be run on Debian), and Bash in POSIX-compatible mode (as it will @@ -230,7 +232,6 @@ Shell * Prefer `for` loops over `while read` loops. * Prefer `grep -c` to `grep | wc -l`. * Prefer `mktemp` over using `$$` to "uniquely" name a temporary file. -* Prefer `printf` over `echo`. * Prefer `sed '/re/!d; s//.../'` to `grep re | sed 's/re/.../'`. * Prefer `sed 'cmd; cmd'` to `sed -e 'cmd' -e 'cmd'`. * Prefer checking exit statuses over output in `if` statements (`if grep From a2428be0f85fa30c0c30dba8d5843e86ff1163f8 Mon Sep 17 00:00:00 2001 From: Jason Draper Date: Wed, 5 Nov 2014 14:44:49 -0500 Subject: [PATCH 077/488] Add first guidelines for Ember --- best-practices/README.md | 9 +++++++++ style/README.md | 5 +++++ 2 files changed, 14 insertions(+) diff --git a/best-practices/README.md b/best-practices/README.md index d41aec92..01f019c1 100644 --- a/best-practices/README.md +++ b/best-practices/README.md @@ -269,3 +269,12 @@ Haskell * Avoid partial functions (`head`, `read`, etc). * Compile code with `-Wall -Werror`. + +Ember +----- +* Avoid using `$` without scoping to `this.$` in views and components. +* Prefer to make model lookup calls in routes instead of controllers (`find`, + `findAll`, etc.). +* Prefer adding properties to controllers instead of models. +* Don't use jQuery outside of views and components. +* Prefer to use predefined `Ember.computed.*` functions when possible. diff --git a/style/README.md b/style/README.md index d7b2218e..7ab43206 100644 --- a/style/README.md +++ b/style/README.md @@ -161,6 +161,11 @@ Backbone * Name the router `App.Router`. * Name views with a `View` suffix. +Ember +----- +* Don't prefix partial templates with a `-` or `_`. +* Don't put a space between the opening handlebars braces and the variable. + Ruby ---- From 061053d73c525b59b701b9e127b033c91c1648d5 Mon Sep 17 00:00:00 2001 From: Jason Draper Date: Thu, 6 Nov 2014 16:10:40 -0500 Subject: [PATCH 078/488] Remove the partial guideline * https://github.com/thoughtbot/guides/commit/a2428be0f85fa30c0c30dba8d5843e86ff1163f8#commitcomment-8468894 --- style/README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/style/README.md b/style/README.md index 7ab43206..688d104a 100644 --- a/style/README.md +++ b/style/README.md @@ -163,7 +163,6 @@ Backbone Ember ----- -* Don't prefix partial templates with a `-` or `_`. * Don't put a space between the opening handlebars braces and the variable. Ruby From 1fa65f6c096261d052d945661a7d826f998d48dc Mon Sep 17 00:00:00 2001 From: Gabe Berke-Williams Date: Mon, 20 Oct 2014 13:27:52 -0400 Subject: [PATCH 079/488] Use `ENV.fetch` instead of `ENV[]` It's easy to forget to set environment variables (e.g. `AWS_SECRET_KEY`) on staging or production. Using `ENV.fetch("AWS_SECRET_KEY")` ensures that missing environment variables raise errors instead of just being `nil`. Heroku boots the app on deploy, which runs the app's initializers. Often, environment variables are set in an initializer. This combination means that if an environment variable isn't set, an error will be raised and Heroku will abort the deploy. This means users can never see a version of the app without the environment variables set. --- best-practices/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/best-practices/README.md b/best-practices/README.md index 01f019c1..8e7417c4 100644 --- a/best-practices/README.md +++ b/best-practices/README.md @@ -93,6 +93,8 @@ Rails * Prefer `Time.current` over `Time.now` * Prefer `Date.current` over `Date.today` * 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. [`.ruby-version`]: https://gist.github.com/fnichol/1912050 [redirects]: http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.30 From ada58245180a9a270028263807ea1e88e1b75bd6 Mon Sep 17 00:00:00 2001 From: Derek Prior Date: Fri, 7 Nov 2014 14:07:58 -0500 Subject: [PATCH 080/488] Add Angular Best Practices A first crack at some Angular best practices arrived at through a seven month angular project and discussions with other teams doing angular work around the company. I'm no angular expert, but these are practices I can remember working well, particularly in the face of other options that *did not* work equally well. --- best-practices/README.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/best-practices/README.md b/best-practices/README.md index 8e7417c4..fab6c370 100644 --- a/best-practices/README.md +++ b/best-practices/README.md @@ -274,9 +274,26 @@ Haskell Ember ----- + * Avoid using `$` without scoping to `this.$` in views and components. * Prefer to make model lookup calls in routes instead of controllers (`find`, `findAll`, etc.). * Prefer adding properties to controllers instead of models. * Don't use jQuery outside of views and components. * Prefer to use predefined `Ember.computed.*` functions when possible. + +Angular +------- + +* [Avoid manual dependency annotations][annotations]. Disable mangling or use a + [pre-processor][ngannotate] for annotations. +* Prefer `factory` to `service`. If you desire a singleton, wrap the singleton + class in a factory function and return a new instance of that class from the + factory. +* Prefer the `translate` directive to the `translate` filter for [performance + reasons][angular-translate]. +* Don't use the `jQuery` or `$` global. Access jQuery via `angular.element`. + +[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 From 0b6487318367d9cba9183c986cad17a79d48df8b Mon Sep 17 00:00:00 2001 From: Derek Prior Date: Fri, 17 Oct 2014 14:30:35 -0400 Subject: [PATCH 081/488] Prefer `reduce` over `inject` `reduce` provides symmetry with our already-preferred `map`. Additionally, it has a more intention-revealing name and is more approachable for people unfamiliar with it. "I am reducing this array to something". `inject` doesn't make the same sense. --- style/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/style/README.md b/style/README.md index 688d104a..d60abcad 100644 --- a/style/README.md +++ b/style/README.md @@ -181,9 +181,9 @@ Ruby * Don't use `self` explicitly anywhere except class methods (`def self.method`) and assignments (`self.attribute =`). * Prefer `detect` over `find`. -* Prefer `inject` over `reduce`. -* Prefer `map` over `collect`. * Prefer `select` over `find_all`. +* Prefer `map` over `collect`. +* Prefer `reduce` over `inject`. * Prefer double quotes for strings. * Prefer `&&` and `||` over `and` and `or`. * Prefer `!` over `not`. From 51e5740eee398b49c8282d8abd96260d1f1c9671 Mon Sep 17 00:00:00 2001 From: Steve Dixon Date: Wed, 10 Dec 2014 18:17:49 +0000 Subject: [PATCH 082/488] Fix typo. --- style/samples/Swift.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/style/samples/Swift.swift b/style/samples/Swift.swift index f6ae5532..b1b9fd0a 100644 --- a/style/samples/Swift.swift +++ b/style/samples/Swift.swift @@ -11,7 +11,7 @@ func yTown(some: Int, withCallback callback: CoolClosure) -> Bool { return CoolClosure(some) } -// It's OK to use $ variable references if the closer is very short and +// It's OK to use $ variable references if the closure is very short and // readability is maintained let cool = yTown(5) { $0 == 6 } From d3d71a7cf2b5173887a386d2d222ccf392f94356 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Wed, 10 Dec 2014 11:11:26 -0800 Subject: [PATCH 083/488] Don't include header comments in swift --- style/samples/Swift.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/style/samples/Swift.swift b/style/samples/Swift.swift index f6ae5532..9f3bc9ba 100644 --- a/style/samples/Swift.swift +++ b/style/samples/Swift.swift @@ -1,3 +1,4 @@ +// Don't include generated header comments import Foundation // or not @@ -41,7 +42,7 @@ func tableView(tableView: UITableView!, cellForRowAtIndexPath indexPath: NSIndex return cell; } } - + return .None } @@ -62,4 +63,3 @@ case let .Success(data): case let .Failure(error): println("An error occured: \(error)") } - From c6f5aabea9f809dfcc24ff1d39ee8660a1bfb657 Mon Sep 17 00:00:00 2001 From: Mike Burns Date: Thu, 6 Nov 2014 19:44:07 +0100 Subject: [PATCH 084/488] Only follow a best practice if you understand it The styles should always be followed, and anything that is a style guideline should be defensible with just "we picked this style." The best practices are things that we've learned through experience and teaching, and therefore cannot be conveyed completely in a pithy guideline. They are reminders for those who understand them, not lessons for those looking to learn. As such, if you do not understand a best practice guideline, then you are simply not the target audience. --- best-practices/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/best-practices/README.md b/best-practices/README.md index fab6c370..c8230584 100644 --- a/best-practices/README.md +++ b/best-practices/README.md @@ -6,6 +6,8 @@ A guide for programming well. General ------- +* These are not to be blindly followed; strive to understand these and ask + when in doubt. * 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. From 9e3db46a7aaa0dd4534e4cb448a6063c482088fa Mon Sep 17 00:00:00 2001 From: Corey Leveen Date: Tue, 4 Nov 2014 19:53:36 -0500 Subject: [PATCH 085/488] Remove typo from long variable name in ruby.rb --- style/samples/ruby.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/style/samples/ruby.rb b/style/samples/ruby.rb index d076b963..74fe73f4 100644 --- a/style/samples/ruby.rb +++ b/style/samples/ruby.rb @@ -60,7 +60,7 @@ def method_with_large_array def invoke_method_with_arguments_on_multiple_lines some_method( - i_am_a_long_variable_name_that_i_will_never_fit_on_one_line_with_others, + i_am_a_long_variable_name_that_will_never_fit_on_one_line_with_others, two, three ) From 3b21ad45e59d3fd797df070dc26d4d16c2f08df6 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Wed, 10 Dec 2014 11:27:43 -0800 Subject: [PATCH 086/488] Add example of type inference in swift functions --- style/samples/Swift.swift | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/style/samples/Swift.swift b/style/samples/Swift.swift index 9f3bc9ba..d0395958 100644 --- a/style/samples/Swift.swift +++ b/style/samples/Swift.swift @@ -53,9 +53,14 @@ enum Response { case Failure(NSError) } -// when the type is known you can let the compiler infer +// When the type is known you can let the compiler infer let response: Response = .Success(NSData()) +func doSomeWork() -> Response { + let data = ... + return .Success(data) +} + switch response { case let .Success(data): println("The response returned successfully \(data)") From 30f9d4110b27afbc5b556bcb5cd6bd5ef7bd7f55 Mon Sep 17 00:00:00 2001 From: Jason Draper Date: Mon, 8 Dec 2014 11:44:19 -0500 Subject: [PATCH 087/488] Ensure that links and buttons are used. * Without the href= in an anchor tag, the hover effect does not work. --- best-practices/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/best-practices/README.md b/best-practices/README.md index c8230584..e4d4bc52 100644 --- a/best-practices/README.md +++ b/best-practices/README.md @@ -187,6 +187,7 @@ HTML * Don't use a reset button for forms. * Prefer cancel links to cancel buttons. +* Use `