From e95e2613220f89f6144e521f45415e77ef0826c9 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Sun, 15 Nov 2015 14:48:47 -0500 Subject: [PATCH 001/330] Add `style/javascript/.jscsrc` Inspired by our Ruby styleguide, which has an accompanying `.rubocop.yml. Adding a JSCS linter to thoughtbot/guides allows us to configure Hound to use our company-wide styleguide: ```yml jscs: enabled: true config_file: https://raw.githubusercontent.com/thoughtbot/guides/master/style/javascript/.jscsrc ``` Uses new [`JSCS@2.6.x`][jscs] rules. [jscs]: https://github.com/jscs-dev/node-jscs/releases/tag/v2.6.0 --- style/javascript/.jscsrc | 80 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 style/javascript/.jscsrc diff --git a/style/javascript/.jscsrc b/style/javascript/.jscsrc new file mode 100644 index 00000000..5f2686b3 --- /dev/null +++ b/style/javascript/.jscsrc @@ -0,0 +1,80 @@ +{ + "esnext": true, + "verbose": true, + "disallowEmptyBlocks": true, + "disallowKeywordsOnNewLine": ["else"], + "disallowMultipleLineBreaks": true, + "disallowMultipleVarDecl": "exceptUndefined", + "disallowNewlineBeforeBlockStatements": true, + "disallowSpaceAfterObjectKeys": true, + "disallowSpaceAfterPrefixUnaryOperators": true, + "disallowSpaceBeforePostfixUnaryOperators": true, + "disallowSpaceBeforeSemicolon": true, + "disallowSpacesInFunction": { + "beforeOpeningRoundBrace": true + }, + "disallowSpacesInsideParentheses": true, + "disallowSpacesInCallExpression": true, + "disallowTrailingComma": false, + "disallowTrailingWhitespace": true, + "disallowVar": true, + "requireArrayDestructuring": true, + "requireBlocksOnNewline": true, + "requireCamelCaseOrUpperCaseIdentifiers": false, + "requireCapitalizedConstructors": true, + "requireCommaBeforeLineBreak": true, + "requireCurlyBraces": [ + "if", + "else", + "for", + "while", + "do", + "try", + "catch", + "switch" + ], + "requireDotNotation": true, + "requireEnhancedObjectLiterals": true, + "requireLineBreakAfterVariableAssignment": true, + "requireObjectDestructuring": true, + "requireOperatorBeforeLineBreak": ["."], + "requireParenthesesAroundArrowParam": false, + "requireSemicolons": true, + "requireSpaceAfterBinaryOperators": true, + "requireSpaceAfterKeywords": [ + "do", + "for", + "if", + "else", + "switch", + "case", + "try", + "void", + "while", + "with", + "return", + "typeof" + ], + "requireSpaceAfterLineComment": true, + "requireSpaceBeforeBinaryOperators": true, + "requireSpaceBeforeBlockStatements": true, + "requireSpaceBeforeKeywords": [ + "else", + "while", + "catch" + ], + "requireSpaceBeforeObjectValues": true, + "requireSpaceBetweenArguments": true, + "requireSpacesInFunction": { + "beforeOpeningCurlyBrace": true + }, + "requireSpacesInConditionalExpression": true, + "requireSpacesInForStatement": true, + "requireSpacesInsideObjectBrackets": "all", + "validateIndentation": 2, + "validateParameterSeparator": ", ", + "validateQuoteMarks": { + "mark": "'", + "escape": true + } +} From 6431419c68021ceb5871cad0fab8c69553b59150 Mon Sep 17 00:00:00 2001 From: Gordon Fontenot Date: Wed, 9 Dec 2015 10:20:43 -0800 Subject: [PATCH 002/330] Add guideline for capitalization in Swift When naming initialisms or acronyms, we should be following the capitalization of the initial letter. BAD: ```swift let Id: String let userId: String ``` GOOD: ```swift let id: String let userID: String ``` --- style/swift/README.md | 2 ++ style/swift/sample.swift | 15 +++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/style/swift/README.md b/style/swift/README.md index 4dca7122..ecbd2d6f 100644 --- a/style/swift/README.md +++ b/style/swift/README.md @@ -12,3 +12,5 @@ Swift * Group computed properties below stored properties * Use a blank line above and below computed properties * Group methods into specific extensions for each level of access control +* When capitalizing acronyms or initialisms, follow the capitalization of the + first letter. diff --git a/style/swift/sample.swift b/style/swift/sample.swift index e16f2670..ab29345e 100644 --- a/style/swift/sample.swift +++ b/style/swift/sample.swift @@ -70,3 +70,18 @@ private extension MyClass { } } + +// MARK: Capitalization + +// Types begin with a capital letter +struct DopeObject { + // if the first letter of an acronym is lowercase, the entire thing should + // be lowercase + let json: AnyObject + + // if the first letter of an acronym is uppercase, the entire thing should + // be uppercase + static func decodeFromJSON(json: AnyObject) -> DopeObject { + return DopeObject(json: json) + } +} From efe0d51bbece75ab0e1d71cd131bd86559a1485f Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Mon, 4 Jan 2016 15:15:37 -0500 Subject: [PATCH 003/330] Clarify how to think about reviewing code --- code-review/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/code-review/README.md b/code-review/README.md index 9f4aba3a..6af128e7 100644 --- a/code-review/README.md +++ b/code-review/README.md @@ -46,7 +46,8 @@ Having Your Code Reviewed Reviewing Code -------------- -Understand why the code is necessary (bug, user experience, refactoring). Then: +Understand why the change is necessary (fixes a bug, improves the user +experience, refactors the existing code). Then: * Communicate which ideas you feel strongly about and those you don't. * Identify ways to simplify the code while still solving the problem. From f6b9866d4a50f5452d8cac09a4211d615a327f7e Mon Sep 17 00:00:00 2001 From: Joshua Clayton Date: Wed, 6 May 2015 17:00:53 -0400 Subject: [PATCH 004/330] Prefer method invocation over instance variables Referring to instance variables in various methods within a class violates the single level of abstraction principle; it applies a specificity that methods using this information need no context of. By preferring method invocation over instance variables, it provides the developer the ability to refer to a concept which can be defined by a method via direct definition or via `attr`, `attr_reader`, or `attr_accessor`. --- style/ruby/README.md | 1 + style/ruby/sample.rb | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/style/ruby/README.md b/style/ruby/README.md index 3423977a..2d05438d 100644 --- a/style/ruby/README.md +++ b/style/ruby/README.md @@ -42,6 +42,7 @@ Ruby * Prefer `protected` over `private` for non-public `attr_reader`s, `attr_writer`s, and `attr_accessor`s. * Order class methods above instance methods. +* Prefer method invocation over instance variables. [trailing comma example]: /style/ruby/sample.rb#L49 [required kwargs]: /style/ruby/sample.rb#L16 diff --git a/style/ruby/sample.rb b/style/ruby/sample.rb index c5244f6f..f1fa9baa 100644 --- a/style/ruby/sample.rb +++ b/style/ruby/sample.rb @@ -77,7 +77,7 @@ def method_without_arguments end def method_that_uses_factory - user = @user_factory.new + user = user_factory.new user.ensure_authenticated! end @@ -87,7 +87,7 @@ def self.class_method protected - attr_reader :foo + attr_reader :foo, :user_factory attr_accessor :bar attr_writer :baz From 87ae60a4fe367d5deb499aedbd7a16fca9d22eb7 Mon Sep 17 00:00:00 2001 From: Geoff Harcourt Date: Wed, 23 Dec 2015 10:40:13 -0500 Subject: [PATCH 005/330] Suggest referencing class by constant in AR macros For ActiveRecord associations where AR can't automatically determine the class for the association, the `class_name` option allows the developer to specify what class should be used. This option can take either the constant of the model class `Person`, or a string that matches the class' name `"Person"`. Using a string to specify this option postpones feedback that the class might be missing. For example, if my model has an association like this where the `Author` class does not yet exist: ```ruby class Post < ActiveRecord::Base has_one :author, class_name: "Person" end ``` Specs will only fail if they interact with the association. If `class_name` is specified as `class_name: Person`, and `Person` does not exist, an uninitialized constant `NameError` exception will stop all execution of the application. There may possibly be some memory-use benefit to creating fewer strings during application initialization, but I think the early failure feedback benefit of this practice provides enough value on its own to recommend it as a best practice. --- best-practices/README.md | 3 +++ style/rails/sample.rb | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/best-practices/README.md b/best-practices/README.md index c3adea33..90313ce3 100644 --- a/best-practices/README.md +++ b/best-practices/README.md @@ -87,6 +87,8 @@ Rails patch level for a project. * Use `_url` suffixes for named routes in mailer views and [redirects]. Use `_path` suffixes for named routes everywhere else. +* Use a [class constant rather than the stringified class name][class constant in association] + for `class_name` options on ActiveRecord association macros. * Validate the associated `belongs_to` object (`user`), not the database column (`user_id`). * Use `db/seeds.rb` for data that is required in all environments. @@ -105,6 +107,7 @@ Rails [redirects]: http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.30 [Spring binstubs]: https://github.com/sstephenson/rbenv/wiki/Understanding-binstubs [prevent tampering]: http://blog.bigbinary.com/2013/03/19/cookies-on-rails.html +[class constant in association]: https://github.com/thoughtbot/guides/blob/master/style/rails/sample.rb Testing ------- diff --git a/style/rails/sample.rb b/style/rails/sample.rb index 7f323880..edee0fd1 100644 --- a/style/rails/sample.rb +++ b/style/rails/sample.rb @@ -1,5 +1,5 @@ class SomeClass - belongs_to :tree + belongs_to :tree, class_name: Plant has_many :apples has_many :watermelons From 7f99e62bf1ab873d5c1c46a938b5d298e914b312 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Wed, 9 Dec 2015 12:15:49 -0500 Subject: [PATCH 006/330] Reccommend `belongs_to :relation, touch: true` Based on @sgrif's recommendation (near 17:19) on [The Bike Shed][bikeshed-41]. Thinking about an application's caching strategy and its implications to the data model is usually put off until some point in the future. `touch`ing `belongs_to` relationships is a straightforward way to pre-emptively configure applications to integrate with Rails' russian doll caching. Rails 5 will batch `touch`-generated DB writes. [bikeshed-41]: http://bikeshed.fm/41 --- best-practices/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/best-practices/README.md b/best-practices/README.md index 90313ce3..678c924e 100644 --- a/best-practices/README.md +++ b/best-practices/README.md @@ -100,6 +100,7 @@ Rails * Use `ENV.fetch` for environment variables instead of `ENV[]`so that unset environment variables are detected on deploy. * [Use blocks][date-block] when declaring date and time attributes in FactoryGirl factories. +* Use `touch: true` when declaring `belongs_to` relationships. [date-block]: /best-practices/samples/ruby.rb#L10 [fkey]: http://robots.thoughtbot.com/referential-integrity-with-foreign-keys From 9d0e56c2e1b571db75a1010869b4328ce739590e Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Wed, 6 Jan 2016 12:52:23 -0500 Subject: [PATCH 007/330] Add `requireTrailingComma` rule to JSCS Add the [`requireTrailingComma`][rule] rule to JSCS configuration. Enforce the presence of trailing comma's in multi-line array and object literal declarations. [rule]: http://jscs.info/rule/requireTrailingComma --- style/javascript/.jscsrc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/style/javascript/.jscsrc b/style/javascript/.jscsrc index 5f2686b3..7963f966 100644 --- a/style/javascript/.jscsrc +++ b/style/javascript/.jscsrc @@ -71,6 +71,9 @@ "requireSpacesInConditionalExpression": true, "requireSpacesInForStatement": true, "requireSpacesInsideObjectBrackets": "all", + "requireTrailingComma": { + "ignoreSingleLine": true + }, "validateIndentation": 2, "validateParameterSeparator": ", ", "validateQuoteMarks": { From c8410d464dd995500debef70815a8a9ceb21305f Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Fri, 8 Jan 2016 13:10:44 -0500 Subject: [PATCH 008/330] Enable Hound Enable [Hound] linting for style guidelines with accompanying code samples. [Hound]: https://houndci.com --- .hound.yml | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .hound.yml diff --git a/.hound.yml b/.hound.yml new file mode 100644 index 00000000..6cd9a402 --- /dev/null +++ b/.hound.yml @@ -0,0 +1,8 @@ +javascript: + enabled: false +jscs: + enabled: true + config_file: style/javascript/.jscsrc +ruby: + enabled: true + config_file: style/ruby/.rubocop.yml From a467a2c6a90984b79a68dfcad76a034c3c878d7f Mon Sep 17 00:00:00 2001 From: Tyson Gach Date: Thu, 17 Dec 2015 11:42:55 -0500 Subject: [PATCH 009/330] Add default SCSS-Lint configuration --- .hound.yml | 3 + style/sass/.scss-lint.yml | 237 ++++++++++++++++++++++++++++++++++++++ style/sass/README.md | 6 +- 3 files changed, 245 insertions(+), 1 deletion(-) create mode 100644 style/sass/.scss-lint.yml diff --git a/.hound.yml b/.hound.yml index 6cd9a402..cb1e5adc 100644 --- a/.hound.yml +++ b/.hound.yml @@ -6,3 +6,6 @@ jscs: ruby: enabled: true config_file: style/ruby/.rubocop.yml +scss: + enabled: true + config_file: style/sass/.scss-lint.yml diff --git a/style/sass/.scss-lint.yml b/style/sass/.scss-lint.yml new file mode 100644 index 00000000..fa1f45f1 --- /dev/null +++ b/style/sass/.scss-lint.yml @@ -0,0 +1,237 @@ +# Up-to-date with SCSS-Lint v0.43.2 + +scss_files: "**/*.scss" + +linters: + BangFormat: + enabled: true + space_before_bang: true + space_after_bang: false + + BemDepth: + enabled: false + max_elements: 1 + + BorderZero: + enabled: true + convention: zero + + ChainedClasses: + enabled: false + + ColorKeyword: + enabled: true + + ColorVariable: + enabled: true + + Comment: + enabled: true + style: silent + + DebugStatement: + enabled: true + + DeclarationOrder: + enabled: true + + DisableLinterReason: + enabled: false + + DuplicateProperty: + enabled: true + + ElsePlacement: + enabled: true + style: same_line + + EmptyLineBetweenBlocks: + enabled: true + ignore_single_line_blocks: true + + EmptyRule: + enabled: true + + ExtendDirective: + enabled: true + severity: warning + + FinalNewline: + enabled: true + present: true + + HexLength: + enabled: true + style: short + + HexNotation: + enabled: false + style: lowercase + + HexValidation: + enabled: true + + IdSelector: + enabled: true + + ImportantRule: + enabled: true + + ImportPath: + enabled: true + leading_underscore: false + filename_extension: false + + Indentation: + enabled: true + allow_non_nested_indentation: false + character: space + width: 2 + + LeadingZero: + enabled: true + style: include_zero + + MergeableSelector: + enabled: true + force_nesting: true + + NameFormat: + enabled: true + allow_leading_underscore: true + convention: hyphenated_lowercase + + NestingDepth: + enabled: true + max_depth: 3 + ignore_parent_selectors: false + + PlaceholderInExtend: + enabled: true + + PropertyCount: + enabled: false + include_nested: false + max_properties: 10 + + PropertySortOrder: + enabled: true + ignore_unspecified: false + min_properties: 2 + separate_groups: false + + PropertySpelling: + enabled: true + extra_properties: [] + disabled_properties: [] + + PropertyUnits: + enabled: true + global: [ + 'ch', 'em', 'ex', 'rem', + 'cm', 'in', 'mm', 'pc', 'pt', 'px', 'q', + 'vh', 'vw', 'vmin', 'vmax', + 'deg', 'grad', 'rad', 'turn', + 'ms', 's', + 'Hz', 'kHz', + 'dpi', 'dpcm', 'dppx', + '%'] + properties: + line-height: [] + + PseudoElement: + enabled: true + + QualifyingElement: + enabled: true + allow_element_with_attribute: false + allow_element_with_class: false + allow_element_with_id: false + + SelectorDepth: + enabled: true + max_depth: 3 + + SelectorFormat: + enabled: false + convention: hyphenated_BEM + + Shorthand: + enabled: true + allowed_shorthands: [1, 2, 3] + + SingleLinePerProperty: + enabled: true + allow_single_line_rule_sets: true + + SingleLinePerSelector: + enabled: true + + SpaceAfterComma: + enabled: true + style: one_space + + SpaceAfterPropertyColon: + enabled: true + style: one_space + + SpaceAfterPropertyName: + enabled: true + + SpaceAfterVariableName: + enabled: true + + SpaceAroundOperator: + enabled: true + style: one_space + + SpaceBeforeBrace: + enabled: true + style: space + allow_single_line_padding: false + + SpaceBetweenParens: + enabled: true + spaces: 0 + + StringQuotes: + enabled: true + style: double_quotes + + TrailingSemicolon: + enabled: true + + TrailingWhitespace: + enabled: true + + TrailingZero: + enabled: true + + TransitionAll: + enabled: true + + UnnecessaryMantissa: + enabled: true + + UnnecessaryParentReference: + enabled: true + + UrlFormat: + enabled: true + + UrlQuotes: + enabled: true + + VariableForProperty: + enabled: false + properties: [] + + VendorPrefix: + enabled: true + identifier_list: base + + ZeroUnit: + enabled: true + + Compass::*: + enabled: false diff --git a/style/sass/README.md b/style/sass/README.md index 566cf28a..302472f0 100644 --- a/style/sass/README.md +++ b/style/sass/README.md @@ -1,6 +1,10 @@ # Sass -[Sample](sample.scss) +- [Sample](sample.scss) +- [Default SCSS-Lint configuration](.scss-lint.yml) + - This configuration aligns with our team-wide guides below. It does _not_, + however, enforce a particular class naming structure (`SelectorFormat`), + which is a team decision to be made on a per-project basis. ## Formatting From d7e06ad3bf775b456d703537d34db6c33dc0222f Mon Sep 17 00:00:00 2001 From: LkeMitchll Date: Fri, 8 Jan 2016 17:17:02 +0000 Subject: [PATCH 010/330] Add link to an explanation of `git rebase -i ...` I did not understand the `git rebase -i ...` interface at first. This link to the Github docs really helped explain the interface. --- protocol/git/README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/protocol/git/README.md b/protocol/git/README.md index c430bdb5..f43a83a2 100644 --- a/protocol/git/README.md +++ b/protocol/git/README.md @@ -47,8 +47,9 @@ Write a [good commit message]. Example format: http://project.management-system.com/ticket/123 -If you've created more than one commit, use a rebase to squash them into -cohesive commits with good messages: +If you've created more than one commit, +[use `git rebase` interactively](https://help.github.com/articles/about-git-rebase/) +to squash them into cohesive commits with good messages: git rebase -i origin/master From 29f844344f048b65ecc0485b41d17de686321574 Mon Sep 17 00:00:00 2001 From: Laila Winner Date: Wed, 21 Oct 2015 11:23:06 -0700 Subject: [PATCH 011/330] Add best practices for building Ruby JSON APIs * Recommend that readers review Heroku's HTTP API Design Guide before developing JSON APIs * Recommend `oj` for parsing JSON and RSpec request specs for integration testing, per iOS on Rails * References: https://github.com/thoughtbot/ios-on-rails/blob/master/book/rails/introduction.md#parsing-incoming-json-requests https://github.com/thoughtbot/ios-on-rails/blob/master/book/rails/creating_a_get_request.md#it-all-starts-with-a-request-spec --- best-practices/README.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/best-practices/README.md b/best-practices/README.md index 678c924e..7a752247 100644 --- a/best-practices/README.md +++ b/best-practices/README.md @@ -335,3 +335,18 @@ Angular [annotations]: http://robots.thoughtbot.com/avoid-angularjs-dependency-annotation-with-rails [ngannotate]: https://github.com/kikonen/ngannotate-rails [angular-translate]: https://github.com/angular-translate/angular-translate/wiki/Getting-Started#using-translate-directive + +Ruby JSON APIs +-------------- + +* Review the recommended practices outlined in Heroku's [HTTP API Design Guide] + before designing a new API. +* Use a fast JSON parser, e.g. [`oj`][oj] +* Write integration tests for your API endpoints. When the primary consumer of + the API is a JavaScript client maintained within the same code base as the + provider of the API, write [feature specs]. Otherwise write [request specs]. + +[HTTP API Design Guide]: https://github.com/interagent/http-api-design +[oj]: https://github.com/ohler55/oj +[feature specs]: https://www.relishapp.com/rspec/rspec-rails/docs/feature-specs/feature-spec +[request specs]: https://www.relishapp.com/rspec/rspec-rails/docs/request-specs/request-spec From 11a3e7c52a7086c0df23f221381653ec86455337 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Tue, 12 Jan 2016 09:10:44 -0500 Subject: [PATCH 012/330] JavaScript: Disallow multiple `var` declaration The `disallowMultipleVarDecl` rule already exists in the `.jscsrc`. Unfortunately, it is invalidly declared. This commit corrects the declaration. [rule]: http://jscs.info/rule/disallowMultipleVarDecl --- style/javascript/.jscsrc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/style/javascript/.jscsrc b/style/javascript/.jscsrc index 7963f966..fa770af4 100644 --- a/style/javascript/.jscsrc +++ b/style/javascript/.jscsrc @@ -4,7 +4,9 @@ "disallowEmptyBlocks": true, "disallowKeywordsOnNewLine": ["else"], "disallowMultipleLineBreaks": true, - "disallowMultipleVarDecl": "exceptUndefined", + "disallowMultipleVarDecl": { + "allExcept": ["undefined"] + }, "disallowNewlineBeforeBlockStatements": true, "disallowSpaceAfterObjectKeys": true, "disallowSpaceAfterPrefixUnaryOperators": true, From ae2f4a36e3f8e386c2768a7d3a503d68ede5a446 Mon Sep 17 00:00:00 2001 From: Joe Ferris Date: Fri, 4 Dec 2015 12:10:06 -0500 Subject: [PATCH 013/330] Avoid rendering issues from async loading We want pages which render quickly, smoothly, and securely. Techniques which disrupt a smooth rendering process should be avoided. --- best-practices/README.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/best-practices/README.md b/best-practices/README.md index 7a752247..7a6e5fed 100644 --- a/best-practices/README.md +++ b/best-practices/README.md @@ -191,6 +191,13 @@ Email [MailView]: https://github.com/37signals/mail_view [ActionMailer Preview]: http://api.rubyonrails.org/v4.1.0/classes/ActionMailer/Base.html#class-ActionMailer::Base-label-Previewing+emails +Web +--- + +* Avoid a Flash of Unstyled Text, even when no cache is available. +* Avoid rendering delays caused by synchronous loading. +* Use https instead of http when linking to assets. + JavaScript ---------- * Use Coffeescript, ES6 with [babel], or another language that compiles to From 0d9b9754a40cb9993f8b98c96d4e8126e7730df3 Mon Sep 17 00:00:00 2001 From: Joe Ferris Date: Thu, 7 Jan 2016 11:13:09 -0500 Subject: [PATCH 014/330] Use an ORDER BY clause for user-visible queries > If sorting is not chosen, the rows will be returned in an unspecified > order. The actual order in that case will depend on the scan and join > plan types and the order on disk, but it must not be relied on. A > particular output ordering can only be guaranteed if the sort step is > explicitly chosen. http://www.postgresql.org/docs/8.3/static/queries-order.html --- best-practices/README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/best-practices/README.md b/best-practices/README.md index 7a6e5fed..f547ff82 100644 --- a/best-practices/README.md +++ b/best-practices/README.md @@ -165,6 +165,9 @@ Postgres * Consider a [partial index] for queries on booleans. * Constrain most columns as [`NOT NULL`]. * [Index foreign keys]. +* Use an `ORDER BY` clause on queries where the results will be displayed to a + user, as queries without one may return results in a changing, arbitrary + order. [`NOT NULL`]: http://www.postgresql.org/docs/9.1/static/ddl-constraints.html#AEN2444 [combines multiple indexes]: http://www.postgresql.org/docs/9.1/static/indexes-bitmap-scans.html From 02d6d780131393b38bcab59d8869c94d6d0ada2e Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Thu, 7 Jan 2016 16:38:38 -0500 Subject: [PATCH 015/330] Prefer testing your Ember application with QUnit https://trello.com/c/VnZehZn1 > Prefer testing your application with [QUnit][ember-test-guides]. [ember-test-guides]: https://guides.emberjs.com/v2.2.0/testing/ --- best-practices/README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/best-practices/README.md b/best-practices/README.md index f547ff82..45220f2c 100644 --- a/best-practices/README.md +++ b/best-practices/README.md @@ -322,6 +322,9 @@ Ember * Prefer dependency injection through `Ember.inject` over initializers, globals on window, or namespaces. ([sample][inject]) * Prefer sub-routes over maintaining state. +* Prefer testing your application with [QUnit][ember-test-guides]. + +[ember-test-guides]: https://guides.emberjs.com/v2.2.0/testing/ Testing From 8514a0aab4737e8dbe01715389f14c879a6d32ba Mon Sep 17 00:00:00 2001 From: Tony DiPasquale Date: Wed, 20 Jan 2016 16:17:05 -0800 Subject: [PATCH 016/330] Update for latest Swift 2+ Since this was written, we got optional chaining with `?` and `println` changed to `print`. --- style/swift/sample.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/style/swift/sample.swift b/style/swift/sample.swift index ab29345e..5c0ede3e 100644 --- a/style/swift/sample.swift +++ b/style/swift/sample.swift @@ -31,13 +31,13 @@ var maybe: Bool? // Use if-let syntax to unwrap optionals if let definitely = maybe { - println("This is \(definitely) here") + print("This is \(definitely) here") } // If the API you are using has implicit unwrapping you should still use if-let func someUnauditedAPI(thing: String!) { if let string = thing { - println(string) + print(string) } } @@ -58,10 +58,10 @@ func doSomeWork() -> Response { switch response { case let .Success(data): - println("The response returned successfully \(data)") + print("The response returned successfully \(data)") case let .Failure(error): - println("An error occured: \(error)") + print("An error occured: \(error)") } // Group methods into specific extensions for each level of access control From 21155bda2afb873231b5f495bd2a67eb09a255c8 Mon Sep 17 00:00:00 2001 From: Gordon Fontenot Date: Tue, 19 Jan 2016 10:33:13 -0700 Subject: [PATCH 017/330] Specify preference for structs over classes Also note that if you _are_ going to use a `class`, you should default to marking it as `final`. --- style/swift/README.md | 2 ++ style/swift/sample.swift | 11 +++++++++++ 2 files changed, 13 insertions(+) diff --git a/style/swift/README.md b/style/swift/README.md index ecbd2d6f..2bb33267 100644 --- a/style/swift/README.md +++ b/style/swift/README.md @@ -5,6 +5,8 @@ Swift * Keep up with the Objective-C style guide above. Will highlight differences here. +* Prefer `struct`s over `class`es wherever possible +* Default to marking classes as `final` * Use `let` whenever possible to make immutable variables * Name all parameters in functions and enum cases * Use trailing closures diff --git a/style/swift/sample.swift b/style/swift/sample.swift index 5c0ede3e..eb1fe727 100644 --- a/style/swift/sample.swift +++ b/style/swift/sample.swift @@ -2,6 +2,17 @@ import Foundation // or not +// MARK: Types + +// Prefer structs over classes +struct User { + let name: String +} + +// When using classes, default to marking them as final +final class MyViewController: UIViewController { +} + // MARK: Closures // Use typealias when closures are referenced in multiple places From 29d4010db5ee59e86f205247721aff4f52c10f24 Mon Sep 17 00:00:00 2001 From: Gordon Fontenot Date: Wed, 20 Jan 2016 16:08:56 -0700 Subject: [PATCH 018/330] Remove callout to Objective-C styleguide in Swift We aren't following many (any?) of the guidelines from Objective-C. There's no real reason to bring attention to it anymore. We should instead look ahead and add guidelines for Swift without the legacy baggage. --- style/swift/README.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/style/swift/README.md b/style/swift/README.md index 2bb33267..f6245547 100644 --- a/style/swift/README.md +++ b/style/swift/README.md @@ -3,8 +3,6 @@ Swift [Sample](sample.swift) -* Keep up with the Objective-C style guide above. Will highlight differences - here. * Prefer `struct`s over `class`es wherever possible * Default to marking classes as `final` * Use `let` whenever possible to make immutable variables From f2e057b4e02875726ec65bb1b6ed38da86eb07ac Mon Sep 17 00:00:00 2001 From: Gordon Fontenot Date: Wed, 20 Jan 2016 16:57:41 -0700 Subject: [PATCH 019/330] Add line length limit for Swift We should probably have _some_ guideline for line length in Swift. 80 feels too short, since we tend to work with fairly verbose APIs. SwiftLint has settled on 100 characters, which seems like a reasonable length. In Xcode you can add a line length guide inside Preferences -> Text Editing. --- style/swift/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/style/swift/README.md b/style/swift/README.md index f6245547..4980693f 100644 --- a/style/swift/README.md +++ b/style/swift/README.md @@ -5,6 +5,7 @@ Swift * Prefer `struct`s over `class`es wherever possible * Default to marking classes as `final` +* Break long lines after 100 characters * Use `let` whenever possible to make immutable variables * Name all parameters in functions and enum cases * Use trailing closures From ccf713bee4fa9cc0a3653d2bacfe0b1c98e92f40 Mon Sep 17 00:00:00 2001 From: Gordon Fontenot Date: Wed, 20 Jan 2016 17:01:53 -0700 Subject: [PATCH 020/330] Use 2 spaces for indentation in Swift This brings Swift inline with the defaults for Ruby and other languages. The benefit here is that while working outside of Xcode (as some of us try to do), we can use a consistent setting for indentation instead of having to special case Swift. From a visual aspect, I've gotten used to the look of 2 spaces, and I think it feels nice to move away from the 4 space indentations of Objective-C. A few of our projects (internal and external) are already using 2 spaces, and I think in _general_ people have liked it more. Note that this has already been proposed, but was shot down because we couldn't come to a real consensus. I'd like to re-poll the team and see where we stand now. --- style/swift/README.md | 1 + style/swift/sample.swift | 36 +++++++++++++++++------------------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/style/swift/README.md b/style/swift/README.md index 4980693f..882776fb 100644 --- a/style/swift/README.md +++ b/style/swift/README.md @@ -6,6 +6,7 @@ Swift * Prefer `struct`s over `class`es wherever possible * Default to marking classes as `final` * Break long lines after 100 characters +* Use 2 spaces for indentation * Use `let` whenever possible to make immutable variables * Name all parameters in functions and enum cases * Use trailing closures diff --git a/style/swift/sample.swift b/style/swift/sample.swift index eb1fe727..c1f15de3 100644 --- a/style/swift/sample.swift +++ b/style/swift/sample.swift @@ -20,7 +20,7 @@ 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) + return CoolClosure(some) } // It's OK to use $ variable references if the closure is very short and @@ -29,11 +29,11 @@ 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 - } + if foo > 5 && foo < 0 { + return true + } else { + return false + } } // MARK: Optionals @@ -42,44 +42,42 @@ var maybe: Bool? // Use if-let syntax to unwrap optionals if let definitely = maybe { - print("This is \(definitely) here") + print("This is \(definitely) here") } // If the API you are using has implicit unwrapping you should still use if-let func someUnauditedAPI(thing: String!) { - if let string = thing { - print(string) - } + if let string = thing { + print(string) + } } // MARK: Enums enum Response { - case Success(NSData) - case Failure(NSError) + case Success(NSData) + case Failure(NSError) } // When the type is known you can let the compiler infer let response: Response = .Success(NSData()) func doSomeWork() -> Response { - let data = ... - return .Success(data) + let data = ... + return .Success(data) } switch response { case let .Success(data): - print("The response returned successfully \(data)") + print("The response returned successfully \(data)") case let .Failure(error): - print("An error occured: \(error)") + print("An error occured: \(error)") } // Group methods into specific extensions for each level of access control private extension MyClass { - func doSomethingPrivate() { - - } + func doSomethingPrivate() { } } // MARK: Capitalization From 591c3cde83e203ccffe8a69cc79f271da5497d92 Mon Sep 17 00:00:00 2001 From: Tony DiPasquale Date: Wed, 20 Jan 2016 15:39:27 -0800 Subject: [PATCH 021/330] Guard against bad `guard` syntax Suggest formatting for dealing with `guard` syntax in Swift. --- style/swift/sample.swift | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/style/swift/sample.swift b/style/swift/sample.swift index c1f15de3..5b8d7807 100644 --- a/style/swift/sample.swift +++ b/style/swift/sample.swift @@ -94,3 +94,23 @@ struct DopeObject { return DopeObject(json: json) } } + +// MARK: Guards + +// One expression to evaluate and short or no return + +guard let singleTest = somethingFailable() else { return } +guard statementThatShouldBeTrue else { return } + +// If there is one long expression to guard or multiple expressions +// move else to next line + +guard let oneItem = somethingFailable(), + let secondItem = somethingFailable2() + else { return } + +// If the return in else is long, move to next line + +guard let something = somethingFailable() else { + return someFunctionThatDoesSomethingInManyWordsOrLines() +} From 306c506c8336b23fa22fb53378c38b81da8c21dc Mon Sep 17 00:00:00 2001 From: Matthew Sumner Date: Fri, 8 Jan 2016 09:49:03 -0500 Subject: [PATCH 022/330] Remove advice to use coffeescript Why: * More projects have been using JavaScript or JavaScript with a transpiler such as babel to get future features of the language. * Leaving clients a project with CoffeeScript adds an additional barrier to finding talent comfortable working on their product. * ES6 transpiling give most of the benefits we enjoy with CoffeeScript, such as classes and object destructuring, without the drawback of a different syntax from JavaScript. * More developers are familiar with JavaScript then CoffeeScript. This PR: * Removes an "Example:" with no following example. --- best-practices/README.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/best-practices/README.md b/best-practices/README.md index 45220f2c..bfdbd8c3 100644 --- a/best-practices/README.md +++ b/best-practices/README.md @@ -203,10 +203,9 @@ Web JavaScript ---------- -* Use Coffeescript, ES6 with [babel], or another language that compiles to - JavaScript +* Use the latest stable JavaScript syntax with a transpiler, such as [babel]. * Include a `to_param` or `href` attribute when serializing ActiveRecord models, - and use that when constructing URLs client side, rather than the ID. Example: + and use that when constructing URLs client side, rather than the ID. [babel]: http://babeljs.io/ From dc03ad58b453fd2d0e702550d8119fb771f51fa4 Mon Sep 17 00:00:00 2001 From: Gordon Fontenot Date: Wed, 20 Jan 2016 15:55:51 -0700 Subject: [PATCH 023/330] Specify when to use () and Void in func signatures We want to avoid ambiguity, especially with our return types. Functions that return `()` can become hard to read when passed as an argument to a higher order function. For example: ```swift func f(g: () -> ()) { } ``` The parens for the return value end up getting lost in the closing parens for the higher order function. Conversely, by using `Void`: ```swift func f(g: () -> Void) { } ``` `Void` stands out as a specific type on its own, and doesn't get lost in a mess of parens. At the same time, `()` is a nice brief indication that a function takes no arguments, and mimics the look of an empty tuple. So if you declare these two functions: ```swift func fa(g: () -> Void) { } func fb(g: (a: T) -> Void) { } ``` There is a visual similarity between the two functions. This is also how Apple prefers to style functions taking or returning `Void`. --- style/swift/README.md | 2 ++ style/swift/sample.swift | 3 +++ 2 files changed, 5 insertions(+) diff --git a/style/swift/README.md b/style/swift/README.md index 882776fb..1424cbb6 100644 --- a/style/swift/README.md +++ b/style/swift/README.md @@ -16,3 +16,5 @@ Swift * Group methods into specific extensions for each level of access control * When capitalizing acronyms or initialisms, follow the capitalization of the first letter. +* When using `Void` in function signatures, prefer `()` for arguments and + `Void` for return types. diff --git a/style/swift/sample.swift b/style/swift/sample.swift index 5b8d7807..e0e445c3 100644 --- a/style/swift/sample.swift +++ b/style/swift/sample.swift @@ -95,6 +95,9 @@ struct DopeObject { } } +// Use () for void arguments and Void for void return types +let f: () -> Void = { } + // MARK: Guards // One expression to evaluate and short or no return From 80750832269d8e7b9996823b0c9538939620fc6a Mon Sep 17 00:00:00 2001 From: Jason Draper Date: Tue, 26 Jan 2016 10:17:24 -0500 Subject: [PATCH 024/330] Recommend against `toggleProperty` Using `toggleProperty` makes tracking down interactions a lot harder. Using explicit actions for what you expect to happens make code easier to maintain and debug. --- best-practices/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/best-practices/README.md b/best-practices/README.md index bfdbd8c3..1b788dc3 100644 --- a/best-practices/README.md +++ b/best-practices/README.md @@ -321,6 +321,7 @@ Ember * Prefer dependency injection through `Ember.inject` over initializers, globals on window, or namespaces. ([sample][inject]) * Prefer sub-routes over maintaining state. +* Prefer explicit setting of boolean properties over `toggleProperty`. * Prefer testing your application with [QUnit][ember-test-guides]. [ember-test-guides]: https://guides.emberjs.com/v2.2.0/testing/ From 6287d1fe9685ee23e9d96d9475f762b1371d4e9d Mon Sep 17 00:00:00 2001 From: Blake Williams Date: Mon, 16 Nov 2015 13:32:20 -0500 Subject: [PATCH 025/330] Use a leading underscore for memoization When using an instance variable prefix the variable with an underscore to discourage direct use of the ivar over the method. --- style/ruby/README.md | 1 + style/ruby/sample.rb | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/style/ruby/README.md b/style/ruby/README.md index 2d05438d..ec617436 100644 --- a/style/ruby/README.md +++ b/style/ruby/README.md @@ -27,6 +27,7 @@ Ruby * Prefer `if` over `unless`. * Use `_` for unused block parameters. * Prefix unused variables or parameters with underscore (`_`). +* Use a leading underscore when defining instance variables for memoization. * Use `%{}` for single-line strings needing interpolation and double-quotes. * Use `{...}` for single-line blocks. Use `do..end` for multi-line blocks. * Use `?` suffix for predicate methods. diff --git a/style/ruby/sample.rb b/style/ruby/sample.rb index f1fa9baa..cae45b82 100644 --- a/style/ruby/sample.rb +++ b/style/ruby/sample.rb @@ -85,6 +85,10 @@ def self.class_method method_body end + def memoized_method + @_memoized_method ||= 1 + end + protected attr_reader :foo, :user_factory From 3de18a7da3aecf4f0f9892445a106dbde90000d2 Mon Sep 17 00:00:00 2001 From: Tyson Gach Date: Wed, 24 Feb 2016 18:36:26 -0500 Subject: [PATCH 026/330] Update SCSS-Lint config to 0.47 --- style/sass/.scss-lint.yml | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/style/sass/.scss-lint.yml b/style/sass/.scss-lint.yml index fa1f45f1..c2c9d576 100644 --- a/style/sass/.scss-lint.yml +++ b/style/sass/.scss-lint.yml @@ -1,7 +1,9 @@ -# Up-to-date with SCSS-Lint v0.43.2 +# Up-to-date with SCSS-Lint v0.47 scss_files: "**/*.scss" +severity: warning + linters: BangFormat: enabled: true @@ -54,7 +56,6 @@ linters: ExtendDirective: enabled: true - severity: warning FinalNewline: enabled: true @@ -109,6 +110,10 @@ linters: PlaceholderInExtend: enabled: true + PrivateNamingConvention: + enabled: false + prefix: _ + PropertyCount: enabled: false include_nested: false @@ -178,6 +183,10 @@ linters: SpaceAfterPropertyName: enabled: true + SpaceAfterVariableColon: + enabled: true + style: one_space + SpaceAfterVariableName: enabled: true From 2d8f13fbf033545724bbd64f17449f9d7aa1aa11 Mon Sep 17 00:00:00 2001 From: Reda Lemeden Date: Wed, 2 Mar 2016 17:05:30 +0100 Subject: [PATCH 027/330] [Swift/ObjC] Prefer strong IBOutlet references --- style/{objective_c => objective-c}/README.md | 4 ++-- style/{objective_c => objective-c}/sample.m | 5 +++-- style/swift/README.md | 1 + style/swift/sample.swift | 2 ++ 4 files changed, 8 insertions(+), 4 deletions(-) rename style/{objective_c => objective-c}/README.md (94%) rename style/{objective_c => objective-c}/sample.m (92%) diff --git a/style/objective_c/README.md b/style/objective-c/README.md similarity index 94% rename from style/objective_c/README.md rename to style/objective-c/README.md index b39e77fc..7ea87c3a 100644 --- a/style/objective_c/README.md +++ b/style/objective-c/README.md @@ -11,8 +11,8 @@ Objective-C * Order `@class` directives alphabetically. * Order `@property` modifiers: memory management, atomicity, writability. * 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. + needed in most cases. +* Prefer strong IBOutlet references. * Prefer `@class` to `#import` when referring to external classes in a public `@interface`. * Prefer `@property` to declaring instance variables. diff --git a/style/objective_c/sample.m b/style/objective-c/sample.m similarity index 92% rename from style/objective_c/sample.m rename to style/objective-c/sample.m index 272b54ac..a5b7248a 100644 --- a/style/objective_c/sample.m +++ b/style/objective-c/sample.m @@ -5,8 +5,9 @@ @interface TBClassName () // Keep @properties grouped together by function -@property (nonatomic, weak) IBOutlet UISearchBar *searchBar; -@property (nonatomic, weak) IBOutlet UITableView *tableView; +// Prefer strong IBOutlet references +@property (nonatomic) IBOutlet UISearchBar *searchBar; +@property (nonatomic) IBOutlet UITableView *tableView; @property (nonatomic) NSManagedObjectContext *managedObjectContext; @property (nonatomic) NSFetchedResultsController *fetchedResultsController; diff --git a/style/swift/README.md b/style/swift/README.md index 1424cbb6..f08f0bde 100644 --- a/style/swift/README.md +++ b/style/swift/README.md @@ -18,3 +18,4 @@ Swift first letter. * When using `Void` in function signatures, prefer `()` for arguments and `Void` for return types. +* Prefer strong IBOutlet references. diff --git a/style/swift/sample.swift b/style/swift/sample.swift index e0e445c3..6c76d9d2 100644 --- a/style/swift/sample.swift +++ b/style/swift/sample.swift @@ -11,6 +11,8 @@ struct User { // When using classes, default to marking them as final final class MyViewController: UIViewController { + // Prefer strong IBOutlet references + @IBOutlet var button: UIButton! } // MARK: Closures From 222af8487ad903dd2dc9f7a8112eeef57ccba8b0 Mon Sep 17 00:00:00 2001 From: Matthew Sumner Date: Fri, 8 Jan 2016 09:39:03 -0500 Subject: [PATCH 028/330] Trailing comma for object literals and arrays Why: * We'd like to explicitly encourage the use of a trailing comma when defining multi line JavaScript object literals or arrays. This PR: * Adds a guideline for using trailing commas and an example. --- style/javascript/README.md | 3 +++ style/javascript/sample.js | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/style/javascript/README.md b/style/javascript/README.md index bb240c0e..3e8efdc4 100644 --- a/style/javascript/README.md +++ b/style/javascript/README.md @@ -19,6 +19,9 @@ JavaScript * Use `const` for declaring variables that will never be re-assigned, and `let` otherwise. * Avoid `var` to declare variables. +* Use a [trailing comma] after each item in a multi-line array or object + literal, including the last item. [template strings]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/template_strings [arrow functions]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions +[trailing comma]: /style/javascript/sample.js#L11 diff --git a/style/javascript/sample.js b/style/javascript/sample.js index f62a3442..42eee962 100644 --- a/style/javascript/sample.js +++ b/style/javascript/sample.js @@ -5,3 +5,8 @@ class Cat { return false; } } + +const somePerson = { + name: 'Ralph', + company: 'thoughtbot', +}; From 98a3ec93805f88beb535ae72585266425de4e5b3 Mon Sep 17 00:00:00 2001 From: Amanda Hill Date: Tue, 8 Mar 2016 13:29:59 -0800 Subject: [PATCH 029/330] add IntelliJ IDEA Java code style settings for thoughtbot's Android projects --- style/android/java/README.md | 13 + .../configs/codestyles/thoughtbotAndroid.xml | 376 ++++++++++++++++++ style/android/java/install.bat | 22 + style/android/java/install.sh | 26 ++ 4 files changed, 437 insertions(+) create mode 100644 style/android/java/README.md create mode 100644 style/android/java/configs/codestyles/thoughtbotAndroid.xml create mode 100644 style/android/java/install.bat create mode 100755 style/android/java/install.sh diff --git a/style/android/java/README.md b/style/android/java/README.md new file mode 100644 index 00000000..5453870e --- /dev/null +++ b/style/android/java/README.md @@ -0,0 +1,13 @@ +Java Code Styles +================ + +IntelliJ IDEA code style settings for thoughtbot's Android projects. + + +Installation +------------ + + * On Unix, run the `install.sh` script. Windows users should use `install.bat` instead. + * Restart IntelliJ if it's running. + * Open IntelliJ Project Settings -> Code Styles, change the code style for the + project to the one you want. diff --git a/style/android/java/configs/codestyles/thoughtbotAndroid.xml b/style/android/java/configs/codestyles/thoughtbotAndroid.xml new file mode 100644 index 00000000..446154c6 --- /dev/null +++ b/style/android/java/configs/codestyles/thoughtbotAndroid.xml @@ -0,0 +1,376 @@ + + + + + diff --git a/style/android/java/install.bat b/style/android/java/install.bat new file mode 100644 index 00000000..dfbe8abe --- /dev/null +++ b/style/android/java/install.bat @@ -0,0 +1,22 @@ +REM Installs thoughtbot's IntelliJ configs into your user configs. +@echo off +echo Installing thoughtbot IntelliJ configs... + +setlocal enableDelayedExpansion + +for /D %%i in (%userprofile%\.AndroidStudio*) do call :copy_config %%i +for /D %%i in (%userprofile%\.IdeaIC*) do call :copy_config %%i +for /D %%i in (%userprofile%\.IntelliJIdea*) do call :copy_config %%i + +echo. +echo Restart IntelliJ or AndroidStudio, go to preferences, and apply 'thoughtbotAndroid'. +exit /b + +REM sub function for copy config files +:copy_config +set config_dir=%~1\config +echo Installing to !config_dir! +xcopy /s configs !config_dir! +echo Done. +echo. +exit /b diff --git a/style/android/java/install.sh b/style/android/java/install.sh new file mode 100755 index 00000000..18e5f08a --- /dev/null +++ b/style/android/java/install.sh @@ -0,0 +1,26 @@ +#!/usr/bin/env bash +# Installs thoughtbot's IntelliJ configs into your user configs. + +echo "Installing thoughtbot IntelliJ configs..." + +CONFIGS=$(dirname "${BASH_SOURCE[0]}")/configs + +for directory in "$HOME"/Library/Preferences/IntelliJIdea* \ + "$HOME"/Library/Preferences/IdeaIC* \ + "$HOME"/Library/Preferences/AndroidStudio* \ + "$HOME"/.IntelliJIdea*/config \ + "$HOME"/.IdeaIC*/config \ + "$HOME"/.AndroidStudio*/config +do + if [[ -d "$directory" ]]; then + + # Install codestyles + mkdir -p "$directory/codestyles" + cp -frv "$CONFIGS/codestyles"/* "$directory/codestyles" + + fi +done + +echo "Done." +echo +echo "Restart IntelliJ or AndroidStudio, go to preferences, and apply 'thoughtbotAndroid'." From 00fe6094394fccb05780b4a030e865a204005c3a Mon Sep 17 00:00:00 2001 From: Joe Ferris Date: Mon, 18 Apr 2016 15:50:50 -0400 Subject: [PATCH 030/330] Fix obsolete Rubocop configuration The DefaultScope cop was removed: https://github.com/bbatsov/rubocop/issues/1895 The TrailingComma cop was split into two: https://github.com/bbatsov/rubocop/pull/2579/commits/a0719f96bc0221cdb99413e033b27c7378520db2 --- style/ruby/.rubocop.yml | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/style/ruby/.rubocop.yml b/style/ruby/.rubocop.yml index a5d47f5f..a14ee718 100644 --- a/style/ruby/.rubocop.yml +++ b/style/ruby/.rubocop.yml @@ -334,12 +334,23 @@ Style/StringLiterals: EnforcedStyle: double_quotes Enabled: true -Style/TrailingComma: - Description: 'Checks for trailing comma in parameter lists and literals.' +Style/TrailingCommaInArguments: + Description: 'Checks for trailing comma in argument lists.' StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#no-trailing-array-commas' EnforcedStyleForMultiline: comma SupportedStyles: - comma + - consistent_comma + - no_comma + Enabled: true + +Style/TrailingCommaInLiteral: + Description: 'Checks for trailing comma in array and hash literals.' + StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#no-trailing-array-commas' + EnforcedStyleForMultiline: comma + SupportedStyles: + - comma + - consistent_comma - no_comma Enabled: true @@ -560,10 +571,6 @@ Rails/Date: such as Date.today, Date.current etc. Enabled: false -Rails/DefaultScope: - Description: 'Checks if the argument passed to default_scope is a block.' - Enabled: false - Rails/FindBy: Description: 'Prefer find_by over where.first.' Enabled: false From f9ce9453eb88575f1ece9cec0a771b7bdc7787c7 Mon Sep 17 00:00:00 2001 From: Mike Burns Date: Thu, 7 Apr 2016 17:02:07 +0200 Subject: [PATCH 031/330] Start a quick "how to ..." section This section is to address the growing amount of internal tools we have built over time; it is an attempt at making them more discoverable in a problem/solution format instead of a long list, plus a quick-start guide. This is in RST format so that we can make use of the table of contents feature. --- README.md | 1 + how-to/README.rst | 55 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 how-to/README.rst diff --git a/README.md b/README.md index 5ca1d2f7..abdb2f8c 100644 --- a/README.md +++ b/README.md @@ -5,6 +5,7 @@ Guides for getting things done, programming well, and programming in style. * [Best Practices](/best-practices) * [Code Review](/code-review) +* [How to](/how-to) * [Protocol](/protocol) * [Communication](/protocol/communication) * [Git](/protocol/git) diff --git a/how-to/README.rst b/how-to/README.rst new file mode 100644 index 00000000..8eeccea9 --- /dev/null +++ b/how-to/README.rst @@ -0,0 +1,55 @@ +How do I ... +============ + +.. contents:: + +... start a new Rails app? +-------------------------- + +Use suspenders_: + +.. code:: sh + + $ gem install suspenders + $ suspenders the-name-of-your-project-here + $ cd the-name-of-your-project-here/ + $ bin/setup + $ rake + +.. _suspenders: https://github.com/thoughtbot/suspenders + +... feature-test a Rails app's Javascript? +------------------------------------------ + +Use capybara-webkit_. In your ``Gemfile``: + +.. code:: ruby + + gem "capybara-webkit" + +In ``spec/support/capybara_webkit.rb`` (for Rspec): + +.. code:: ruby + + Capybara.javascript_driver = :webkit + + Capybara::Webkit.configure do |config| + config.block_unknown_urls + end + +When writing a spec, you must set the ``:js`` flag for that test to make use of +capybara-webkit. For example, in ``spec/features/user_signs_in_spec.rb``: + +.. code:: ruby + + feature "Authentication", :js do + scenario "A user signing in" do + create(:user, email: "me@example.com", password: "sekrit") + + sign_in_as email: "me@example.com", password: "sekrit" + + expect(page).to have_text("Welcome!") + end + end + +.. _capybara-webkit: https://github.com/thoughtbot/capybara-webkit From 5deeaaa2006a7f895f697cc99668eadaa4abe693 Mon Sep 17 00:00:00 2001 From: Bernerd Schaefer Date: Wed, 23 Mar 2016 09:03:13 -0700 Subject: [PATCH 032/330] Prefer private for non-public accessors We added a preference for `protected` over `private` for non-public accessors in [#190]. The primary motivation was because Ruby emitted warnings when declaring private attributes. In Ruby versions 2.3.0 and greater, Ruby no longer warns about private attributes, so there's no longer a need to use `protected` for them. [#190]: https://github.com/thoughtbot/guides/pull/190 --- style/ruby/README.md | 4 ++-- style/ruby/sample.rb | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/style/ruby/README.md b/style/ruby/README.md index ec617436..1cf9db61 100644 --- a/style/ruby/README.md +++ b/style/ruby/README.md @@ -40,8 +40,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. +* Prefer `private` over `protected` for non-public `attr_reader`s, + `attr_writer`s, and `attr_accessor`s. * Order class methods above instance methods. * Prefer method invocation over instance variables. diff --git a/style/ruby/sample.rb b/style/ruby/sample.rb index cae45b82..32be77e2 100644 --- a/style/ruby/sample.rb +++ b/style/ruby/sample.rb @@ -89,14 +89,12 @@ def memoized_method @_memoized_method ||= 1 end - protected + private attr_reader :foo, :user_factory attr_accessor :bar attr_writer :baz - private - def complex_condition? part_one? && part_two? end From 083c9edeb87159866490c951070ef6fc4b8a04fb Mon Sep 17 00:00:00 2001 From: Rob Wierzbowski Date: Mon, 9 May 2016 09:43:34 -0400 Subject: [PATCH 033/330] Put remote devs on equal footing for code review Instead of promoting in-person communication, we can promote synchronous communication. This matches the intent without ignoring the increasing chance that a developer is remote. --- code-review/README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/code-review/README.md b/code-review/README.md index 6af128e7..54272a62 100644 --- a/code-review/README.md +++ b/code-review/README.md @@ -20,8 +20,9 @@ Everyone * Don't use sarcasm. * Keep it real. If emoji, animated gifs, or humor aren't you, don't force them. If they are, use them with aplomb. -* Talk in person if there are too many "I didn't understand" or "Alternative - solution:" comments. Post a follow-up comment summarizing offline discussion. +* Talk synchronously (e.g. chat, screensharing, in person) if there are too many + "I didn't understand" or "Alternative solution:" comments. Post a follow-up + comment summarizing the discussion. Having Your Code Reviewed ------------------------- From 8d747c5c45a21160775033bb88f8ec67353d8531 Mon Sep 17 00:00:00 2001 From: Gordon Fontenot Date: Fri, 12 Feb 2016 14:52:27 -0600 Subject: [PATCH 034/330] Prefer protocol conformance to object inheritance Protocol conformance in Swift 2+ is a much more powerful form of polymorphism than object inheritance and so should be the preferred method. Among other things, using protocols means that you don't need to care about the kind of object (struct, enum, or class), which puts way fewer restrictions on your larger project architecture decisions. Conversely, since object inheritance can only be achieved with classes, they force you to make certain decisions about your application architecture that might have adverse side effects. --- style/swift/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/style/swift/README.md b/style/swift/README.md index f08f0bde..05063b28 100644 --- a/style/swift/README.md +++ b/style/swift/README.md @@ -5,6 +5,7 @@ Swift * Prefer `struct`s over `class`es wherever possible * Default to marking classes as `final` +* Prefer protocol conformance to class inheritance * Break long lines after 100 characters * Use 2 spaces for indentation * Use `let` whenever possible to make immutable variables From b11f416faa3cdbbded833829d7a036190e7e8a71 Mon Sep 17 00:00:00 2001 From: Adam Sharp Date: Fri, 4 Mar 2016 13:50:47 +1100 Subject: [PATCH 035/330] Add Swift guideline recommending weak references are evaluated once If a weak reference is evaluated multiple times within the same scope, it's technically possible for the reference to change between `nil` and non-`nil` between references. In order to ensure that valid references remain valid for the entire scope, recommend that weak references are evaluated only once. --- style/swift/README.md | 2 ++ style/swift/sample.swift | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/style/swift/README.md b/style/swift/README.md index 05063b28..c3db66be 100644 --- a/style/swift/README.md +++ b/style/swift/README.md @@ -20,3 +20,5 @@ Swift * When using `Void` in function signatures, prefer `()` for arguments and `Void` for return types. * Prefer strong IBOutlet references. +* Avoid evaluating a weak reference multiple times in the same scope. + Strongify first, then use the strong reference. diff --git a/style/swift/sample.swift b/style/swift/sample.swift index 6c76d9d2..ab4add3a 100644 --- a/style/swift/sample.swift +++ b/style/swift/sample.swift @@ -38,6 +38,13 @@ let cool = yTown(5) { foo in } } +// Strongify weak references in async closures +APIClient.getAwesomeness { [weak self] result in + guard let `self` = self else { return } + self.stopLoadingSpinner() + self.show(result) +} + // MARK: Optionals var maybe: Bool? From f3a3cbff108633f0d7e1baaa4bded160827ab7a7 Mon Sep 17 00:00:00 2001 From: Amanda Hill Date: Wed, 11 May 2016 15:34:59 -0700 Subject: [PATCH 036/330] Update Java codeStyleSettings for method chained method call wrapping --- .../configs/codestyles/thoughtbotAndroid.xml | 29 +++++++------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/style/android/java/configs/codestyles/thoughtbotAndroid.xml b/style/android/java/configs/codestyles/thoughtbotAndroid.xml index 446154c6..38ab4b8a 100644 --- a/style/android/java/configs/codestyles/thoughtbotAndroid.xml +++ b/style/android/java/configs/codestyles/thoughtbotAndroid.xml @@ -156,17 +156,12 @@ -