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
- 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