Commit Messages

Commit messages should follow Ryan Davis’ (possibly some of my own flair) standard of:

The goal of this is to programmatically generate our changelog for each release so we can do something like $ rake generate:changelog. This will then be used to create the release description so that downstreamers will know what we are doing / to expect.

Commit messages should be in the imperative: “Fix bug” not “Fixes, Fixed bug”

http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html

Formatting

• Code should be only <= 80 characters wide. This helps:
  • Keep the code readable
  • Helps people with various editors / font sizes.
  • Keeps you from chaining too many methods together E.g. Foo.this(param).another(/hi/, ‘mom’).thing.what(the, heck).is.going.to(happen, if, this, keeps).going
  • Helps you see patterns for refactoring / DRYing your code.
• Make your code as human readable as possible (Think 6months from now when you have to edit it again). This is especially true for local variable names, loop var names, function names (bad example: def do_the_rest_of_the_stinkin_query), etc.
• NEVER add trailing whitespace. Its just extra bytes / annoyance for absolutely no reason. Please remove all trailing whitespace before committing. Set your editor to not add whitespace accordingly.

Language-Specific Guidelines

• http://zenspider.com/Languages/Ruby/QuickRef.html#general-tips (specifically)
  • align hashes and arrays on separate lines
  • Use parens to disambiguate, otherwise avoid them.
• I try and alphabetize method names to optimize readability and organization.
  • group methods by type (class, instance, private)

Code Reviewing

There are many lists out there to help us start to think about how to properly review someone’s code. This assumes those lists are understood and hopes to focus it a little more to what we should do at YP specifically. The goal is to drive to quality across the board as an organization.

• All code being checked in should be reviewed. Grab your neighbor real quick and go over the change. If it takes too long to explain, you probably are committing too much at once. This assists in cross pollination.
• All code being checked in should have a corresponding TICKET_NUMBER. This can feel pedantic, but assists greatly with DRYing up communication, documentation, deployments, and all of testing.
• Reviewer should feel free to be critical and should ask questions until the what, why, and how are answered.
• All code should have unit testing and reviewer should scrutinize the testing as well to see if it is sufficient or porous. Think of use cases, edge cases, etc.
• Once review is complete, the ticket should have the reviewers name attached to it in the “Reviewed By” section.