We're so glad you're thinking about contributing to an 18F open source project! If you're unsure about anything, just ask — or submit the issue or pull request anyway. The worst that can happen is you'll be politely asked to change something. We love all friendly contributions.
We want to ensure a welcoming environment for all of our projects. Our staff follow the 18F Code of Conduct and all contributors should do the same.
We encourage you to read this project's CONTRIBUTING policy (you are here), its LICENSE, and its README.
If you have any questions or want to read more, check out the 18F Open Source Policy GitHub repository, or just shoot us an email.
Below are rules we strive to follow to achieve maintainable and consistent code.
-
Write your commit message summary in the imperative: "Fix bug" and not "Fixed bug" or "Fixes bug." This convention matches up with commit messages generated by commands like
git merge
andgit revert
. -
Under the summary, start by explaining why this change is necessary, and add details to help the person reviewing your code understand what your pull request is about.
-
If the pull request fixes a GitHub issue, mention it at the bottom using GitHub's syntax, such as
Fixes #123
.
Example:
Load seed using before(:suite) in RSpec config
**Why**:
- Loading the seed in a `before(:each)` block results in an unnecessary
database call before every single test, slowing down the test suite,
and making development less efficient.
**How**:
- Use `before(:suite)` instead, since the data that is loaded is not
meant to change, and so that only one database call is made.
- To prevent the data from being wiped out after each spec, configure
Database Cleaner to ignore those static tables.
Fixes #123
Note that we use Overcommit to enforce some of the commit message rules.
If this is your first time contributing to this repo, you will need to
sign your Overcommit configuration by running overcommit --sign
before
being able to run git commit
. See the Security section in the Overcommit
README for more details.
-
Rubocop or Reek offenses are not disabled unless they are false positives. If you're not sure, please ask a teammate.
-
Related methods in the same class are in descending order of abstraction. This is best explained through this video: https://youtu.be/0rsilnpU1DU?t=554
-
Compound conditionals are replaced with more readable methods that describe the business rule. For example, a conditional like
user_session[:personal_key].nil? && current_user.personal_key.present?
could be extracted into a method calledcurrent_user_has_already_confirmed_their_personal_key?
. Another example is explained in this video: https://youtu.be/0rsilnpU1DU?t=40s -
Service Objects should usually only have one public method, usually named
call
. This mostly applies to classes that perform a specific task, unlike Presenters, View Objects, and Value Objects, for example. Read 7 Patterns to Refactor Fat ActiveRecord Models for a good overview of the different types of classes used in Rails.References:
- https://medium.com/selleo/essential-rubyonrails-patterns-part-1-service-objects-1af9f9573ca1
- https://multithreaded.stitchfix.com/blog/2015/06/02/anatomy-of-service-objects-in-rails/
- https://hackernoon.com/the-3-tenets-of-service-objects-c936b891b3c2
- http://katafrakt.me//2018/07/04/writing-service-objects/
- https://pawelurbanek.com/2018/02/12/ruby-on-rails-service-objects-and-testing-in-isolation/
Please follow our Code Review guidelines. Glen Sanford's thoughts on code reviews are also well worth reading.
- Prioritize code reviews for the current sprint above your other work
- Review pull requests for the current sprint within 24 hours of being opened
- Keep pull requests as small as possible, and focused on a single topic
- Once a pull request is good to go, the person who opened it squashes related commits together, merges it, then deletes the branch.
- Practical Object-Oriented Design in Ruby
- 99 Bottles of OOP
- Sandi Metz blog
- Sandi Metz talks
- Learn Clean Code
- Ruby Science
- Ruby Tapas
- Master the Object-Oriented Mindset in Ruby and Rails
- Refactoring Rails
- Growing Rails Applications in Practice
- The 30-Day Code Quality Challenge
- SourceMaking
This project is in the public domain within the United States, and copyright and related rights in the work worldwide are waived through the CC0 1.0 Universal public domain dedication.
All contributions to this project will be released under the CC0 dedication. By submitting a pull request, you are agreeing to comply with this waiver of copyright interest.