Best Practices

Guides for getting things done, programming well, and programming in style.

• General
• Ruby
• Database
• Background Jobs
• Testing
• Git
• Code Review

A note on the language:

• Use is a positive instruction. ^[link]
• Prefer indicates a better option and its alternative to watch out for. ^[link]
• Avoid means don't do it unless you have good reason. ^[link]
• Don't means there's never a good reason. ^[link]

General

• Keep documentation and configuration samples up to date. ^[link]
• Have your changes covered by tests. ^[link]
• Prefer a single log message over multiple messages. ^[link]
• Prefer to order methods so that caller methods are earlier in the file than the methods they call. ^[link]
• Prefer to use ChatOps to automate processes. ^[link]
• Prefer to use gitflow-avh with our custom hooks while collaborating with the QA team. ^[link]
• Avoid object types in names (user_array, email_method, CalculatorClass, ReportModule). ^[link]
• Avoid monkey-patching; try to contribute to open source if you have to. ^[link]
• Avoid swallowing exceptions and failing silently. ^[link]
• Don't extract you code into a library if it used only in a single project and you are not going to publish it. ^[link]
• Use perform method name in backgound job, use_case and command classes ^[link]

Ruby

• Use Ruby Style Guide by default, except the guides below. ^[link]
• Use the .ruby-version file convention to specify the Ruby version for a project. ^[link]
• Use maximum 120 characters in a single line. ^[link]
• Use Class.new(StandardError) instead of inheritance to declare a single-line error class. ^[link]
• Prefer to use Dependency Injection pattern instead of specifying a hardcoded behavior for the environment. ^[link]
• Prefer to use acronyms as words in names (XmlHttpRequest, not XMLHTTPRequest). ^[link]
• Prefer to log exception with a trace (logger.error("#{e.inspect}\n#{e.backtrace}")). ^[link]
• Prefer private when indicating scope; use protected only with comparison methods like ==. ^[link]
• Prefer {} for multiline blocks in RSpec let, let! and ActiveRecord validations, scopes. ^[link]
• Prefer to freeze all mutable constants. ^[link]
• Prefer to use # frozen_string_literal: true with Ruby >= 2.3. ^[link]
• Prefer to use keyword arguments if there are more than 2 arguments. ^[link]
• Prefer single-quoted strings when you don't need string interpolation or special symbols such as \n, ', etc. ^[link]
• Prefer to use the . on the second line when continuing a chained method invocation on another line. ^[link]
• Prefer to use joins(:relation) for ActiveRecord models instead of writing SQL queries manually. ^[link]
• Don't create an association inside ActiveRecord model, if you are not going to use it right now. ^[link]
• Don't use spaces after { and before } for hash literals. ^[link]
• Don't use Timeout. ^[link]
• Don't use reassignment of constant ^[link]

Database

• Avoid having breaking changes in DB migrations, they should work with both versions of code. ^[link]
• Don't use the code from application in migrations if it may be changed. ^[link]

Background Jobs

• Use IDs, not ActiveRecord objects for cleaner serialization, then re-find the ActiveRecord object in the perform method. ^[link]
• Use two public methods: perform, perform_async. ^[link]

Testing

• Use not_to instead of to_not in RSpec expectations. ^[link]
• Prefer to follow the betterspecs.org rules. ^[link]
• Prefer to use instance_double instead of double with RSpec. ^[link]
• Prefer to use FactoryGirl build_stubbed instead of build to avoid creating associations. ^[link]
• Avoid using associations in FactoryGirl, use them in trait. ^[link]
• Don't test private methods. ^[link]

Mocks

• Avoid using mocks at all when feasible. ^[link]
• Prefer instance_double(SomeClass) over double(method: return_value). ^[link]
• Enable verify_partial_doubles and verify_doubled_constant_names. ^[link]
• Prefer mocking SomeClass.new over any_instance_of(SomeClass). ^[link]

Git

• Use simple verbs in commit messages (Fix, Add, Refactor). ^[link]
• Use task number at the beginning of each commit message (HER-666 Delete app directory). ^[link]
• Use task number in branch name if you're using gitflow (feature/SCD-777_lucky_ticket). ^[link]
• Use English characters, digits, -, _, / and . in branch names. ^[link]
• Use PR to merge your branch. ^[link]
• Don't use force push if you opened a PR with reviewers. ^[link]

Code Review

• Be humble (I'm not sure, in my opinion). ^[link]
• Ask questions, but be explicit and describe your point. ^[link]
• Try to review a complete PR within one day. ^[link]
• Try to respond to every reviewer's comment. ^[link]
• Talk synchronously (e.g. chat, in person) if there are a lot of disputes, misunderstandings, etc. Post a follow-up comment summarizing the discussion. ^[link]
• Don't merge PR if it broke tests on CI. ^[link]
• Don't resolve task and don't merge PR without at least one approval. ^[link]
• Avoid changes irrelevant to the task. Open separate PR for refactoring changes. ^[link]

Contributing

Bug reports and pull requests are welcome on GitHub. If you want to contribute, please read CONTRIBUTING.md.

License

The gem is available as open source under the terms of the MIT License.