The Rules

TL;DR

1. Branch
2. PR
3. Review
4. Test
5. Green

Design Guidelines

• Git
  • Rebase often (on master)
    • As a rule of thumb, if you're not confident with git-rebase, you can keep your branch updated using a simple git-merge from master
• Ruby
  • No OpenStruct
• Rails
  • Rspec 3 syntax
  • Views should always use a presenter rather than raw AR or Spree objects
  • Never use hardcoded URLs
  • Try and treat Spree as closed under the Open/Closed Principle
    • Avoid adding new functionality by monkey patching Spree
    • Prefer adding new classes for new behaviours, and hooking it in as surgically as possible.
  • Use modules for new objects like services and concerns, and folders for each module
    • e.g. Services::Marketing::UserVisits would live in app/services/marketing/user_visits.rb

Style Guide

We loosely follow the Github Ruby Styleguide.

We also have a Commit Styleguide.

Branch

• All work should be on a branch
• Branch names must include JIRA ticket numbers and optionally Github Issue Numbers prepended with a '#'
  • For extra points, name the branch to annotate the type of work and ticket number being resolved

Examples:

• bugfix/WEBSITE-123/broken-redirects-for-guests
• feature/WEBSITE-123/new-cart-logo#2001
• ops/WEBSITE-808/cloud-66-postgres-tweaks
• feature/WEBSITE-1304/branch-name#2304

Types of branches

feature -> feature
bugfix  -> bugfix
copy    -> update to site copy / content
refine  -> refactoring code / style / documentation
test    -> adding missing tests 
ops     -> infrastructure / ops related changes

PR

• Merges should be managed using a github Pull Request
• Before finishing a PR, rebase on master

Review

At least one other person should review the PR before merge.

A review should ideally involve actually checking out and running the code, sanity checking it before merge.

Test

Tests are good.

• All new green-fields work must have tests.
• Older code should be tested at the model, service level for any fixes or enhancements where feasible.
• Routes should be tested (we have a lot of complexity and keep finding bugs).
• Controller should be tested where it is feasible.

To run every test, including engines', use the bundle exec rake spec command

Green

The build should be green before you merge. The build should be green after you merge.