Writing reviewable code
One important aspect of our team's culture is the same, regardless of language or platform: code review. All non-experimental code we write as a team goes through code review before it can be landed in any shared git repository.
Pre-reading: Phabricator's "Writing reviewable code" article.
Summarizing the important points:
Make many small commits
Good examples:
- A single line fix for a bug.
- A new class with stub APIs. Follow-up diffs flesh out one feature at a time.
Write documentation early
Much of what we build is designed to be used by other developers. While self-documenting logic is a good thing to strive for, APIs rarely, if ever, are truly self-documenting.
Include documentation with any new API.
Break big changes in to smaller changes
Phabricator makes it possible to mark a diff as depending on another diff.
Here's one workflow for doing work on two dependent diffs:
git fetch # Get the latest from origin
arc feature foo # Start a new feature, "foo", based off of origin/develop
# make changes
git commit -am "Some commit"
arc diff # Creates diff 1
# Code is out for review....now you want to make more changes
arc feature bar foo # Start a new feature, "bar", tracking the "foo" branch
# make changes
git commit -am "Another commit"
arc diff foo # Creates diff 2
If you need to make changes to foo:
git checkout foo
# make changes
git commit -am "Additional changes
arc diff # Updates diff 1
# bar is now pointing to an old sha, let's update it
git checkout bar
git rebase foo
arc diff foo # Updates diff 2