CONTRIBUTING

---

Table of contents:

1. General
2. Branches & Pull Requests
3. Code Style
4. Common Utilities
5. Opinionated Changes

---

General

• Do not commit directly to main
• Do not commit to stable unless you are a maintainer following the process for cutting releases.
• Pull requests require at least one +1 by a maintainer before they can be merged. There are some exceptions, which can be merged without a +1, but still require a PR with passing tests:
  • Small docs only changes. Use your judgement, typos are fine, larger updates can probably benefit from review
  • Fixing specs which are currently failing on main - only with small and urgent fixes
  • Updates to org specific .yml settings files.
• Read these git commit message guidelines and do your best to follow them.
• See the issue template and PR template.
• It is your responsibility to ensure tests and rubocop pass on your PR branch. It’s fine to push your branch to github and use CircleCI to verify this, but try to have CircleCI green before sending a PR for review.

Branches & Pull Requests

• We generally follow a GitFlow approach for feature branches, outlined in this helpful article from Atlassian.
  • The one notable exception is that instead of having a develop branch with releases to master, we have a main branch with releases to stable.
• When making changes, please start from main, and create atomic feature branches, namespaced like: feature/new-csl-webhooks or fix/cache-counts.
• To make the life of maintainers easier, we use github labels to mark PRs when they are ready for review. Please use:
  • ready-for-main-merge - Please add this to a PR when it’s ready to be reviewed by maintainers and merged into main. We’ll assume that PRs without this are WIP / serve some other purpose, and therefore shouldn’t be reviewed.
  • feedback-please - This indicates that you would like feedback on the approach in the PR but don’t consider it ready for merging
  • depends-unmerged - This PR depends on another PR being merged and so can’t be reviewed yet.
  • ongoing - This PR requires additional feedback and/or more work is being done on it, so won’t be purged if inactive for more than 28 days.
• Many organisations don’t need to track main closely, and will choose instead to track stable. In this case, you’ll still create feature branches off of main and send PRs to main. However, if you need a fix quickly, before the next scheduled release, there are three approaches you can take:
  • Ask for a micro-release: create a feature branch off of main just as you normally would, but ask the maintainers to do a quick release containing just this fix so that stable gets the changes quickly.
  • Use a short-lived deployment branch for your organisation: Branch off of main and send PRs with atomic commits as you usually would. Then re-apply those changes against your own version of stable (e.g. git checkout stable && git checkout -b deploy/my-org). Run your own deploy branch until the next release comes out, and then switch back to stable.
  • Hotfixing: Starting from stable, create a new branch like hotfix/my-fix, send the PR to stable, and message the maintainers to get it merged ASAP. Note: This should be used only in extreme circumstances.
• Do not send pull requests with unrelated features and fixes in them. If you have diverged significantly from main, re-apply your changes to main, one feature branch at a time.
• If your branch has a lot of noisy commits (perhaps from testing on your heroku staging app), please open a new branch and squash the commits into something more reasonable.
• We will reject PRs that lack tests, introduce new features without documentation, or lack the proper updates to the DB seed.
• Any PRs that are inactive for more than 28 days will be closed. You are welcome to reopen it if you have done any additional work on it. Mark PRs that have a longer cycle as ongoing

Code Style

• Ensure your editor is setup with an EditorConfig plugin, and recognizes the local .editorconfig file.
• Use the Ruby Style Guide.
• Refer to Better Specs for guidelines on how to write readable tests.
• We use Rubocop to help stick to this, (aside from some changes and exclusions in .rubocop.yml). It’s run on CI, and will fail if there are any issues. You can run it locally using bundle exec rubocop.
• Do not use ENV['...'] directly in application code. Use Settings instead.
• Do not output debugging information using puts, use Rails.logger.debug instead

• When typing SQL strings, use squiggly heredoc with an SQL terminating sequence. The benefit of doing this is that code editors will understand that the string is SQL and properly highlight it as such. Read more about the squiggly heredoc.

  # bad
  sql = 'SELECT email FROM members'
  
  # bad
  sql = "
    SELECT email FROM members
  "
  
  # good
  sql = <<~SQL
    SELECT email FROM members
  SQL
  

Common Utilities

• Whenever you’re recording a member action, use Member#record_action
• Whenever you’re updating member details, use UpsertMember#call (which is called by Member#record_action)
• To clean emails, use Cleanser#cleanse_email
• To clean phone numbers, use PhoneNumber#standardise_phone_number

Opinionated Changes

• If you are making changes that might break someone else’s setup, consult the appropriate channel on Slack, or ask the listserv.
• If you are adding a feature that might be disruptive for others, add a setting to enable or disable it, and make the default be disabled, to avoid disrupting other setups.
• If you are adding features that require some 3rd-party API keys or other setup to work properly, make sure the feature is not visible unless it has been configured. On private (admin) screens, it’s acceptable to show the feature as long as it presents a friendly error message when people try to use it.