3c58389083
Based on a draft provided by Han-Wen Nienhuys (hanwen@google.com) Change-Id: Ib593bf6a5c78b9b5c44b062a205eb03fef926b19
130 lines
5.3 KiB
Plaintext
130 lines
5.3 KiB
Plaintext
= Use Gerrit to Be a Rockstar Programmer
|
||
|
||
== Overview
|
||
|
||
The term _rockstar_ is often used to describe those talented programmers who
|
||
seem to work faster and better than everyone else, much like a composer who
|
||
seems to effortlessly churn out fantastic music. However, just as the
|
||
spontaneity of masterful music is a fantasy, so is the development of
|
||
exceptional code.
|
||
|
||
The process of composing and then recording music is painstaking — the artist
|
||
records portions of a composition over and over, changing each take until one
|
||
song is completed by combining those many takes into a cohesive whole. The end
|
||
result is the recording of the best performance of the best version of the
|
||
song.
|
||
|
||
Consider Queen’s six-minute long Bohemian Rhapsody, which took three weeks to
|
||
record. Some segments were overdubbed 180 times!
|
||
|
||
Software engineering is much the same. Changes that seem logical and
|
||
straightforward in retrospect actually required many revisions and many hours
|
||
of work before they were ready to be merged into a code base. A single
|
||
conceptual code change (_fix bug 123_) often requires numerous iterations
|
||
before it can be finalized. Programmers typically:
|
||
|
||
* Fix compilation errors
|
||
* Factor out a method, to avoid duplicate code
|
||
* Use a better algorithm, to make it faster
|
||
* Handle error conditions, to make it more robust
|
||
* Add tests, to prevent a bug from regressing
|
||
* Adapt tests, to reflect changed behavior
|
||
* Polish code, to make it easier to read
|
||
* Improve the commit message, to explain why a change was made
|
||
|
||
In fact, first drafts of code changes are best kept out of project history. Not
|
||
just because rockstar programmers want to hide sloppy first attempts at making
|
||
something work. It's more that keeping intermediate states hampers effective
|
||
use of version control. Git works best when one commit corresponds to one
|
||
functional change, as is required for:
|
||
|
||
* git revert
|
||
|
||
* git cherry-pick
|
||
|
||
* link:https://www.kernel.org/pub/software/scm/git/docs/git-bisect-lk2009.html[git bisect]
|
||
|
||
|
||
[[amending]]
|
||
== Amending commits
|
||
|
||
Git provides a mechanism to continually update a commit until it’s perfect: use
|
||
`git commit --amend` to remake (re-record) a code change. After you update a
|
||
commit in this way, your branch then points to the new commit. However, the
|
||
older (imperfect) revision is not lost. It can be found via the `git reflog`.
|
||
|
||
|
||
[[review]]
|
||
== Code review
|
||
|
||
At least two well-known open source projects insist on these practices:
|
||
|
||
* link:http://git-scm.com/[Git]
|
||
* link:http://www.kernel.org/category/about.html/[Linux Kernel]
|
||
|
||
However, contributors to these projects don’t refine and polish their changes
|
||
in private until they’re perfect. Instead, polishing code is part of a review
|
||
process — the contributor offers their change to the project for other
|
||
developers to evaluate and critique. This process is called _code review_ and
|
||
results in numerous benefits:
|
||
|
||
* Code reviews mean that every change effectively has shared authorship
|
||
|
||
* Developers share knowledge in two directions: Reviewers learn from the patch
|
||
author how the new code they will have to maintain works, and the patch
|
||
author learns from reviewers about best practices used in the project.
|
||
|
||
* Code review encourages more people to read the code contained in a given
|
||
change. As a result, there are more opportunities to find bugs and suggest
|
||
improvements.
|
||
|
||
* The more people who read the code, the more bugs can be identified. Since
|
||
code review occurs before code is submitted, bugs are squashed during the
|
||
earliest stage of the software development lifecycle.
|
||
|
||
* The review process provides a mechanism to enforce team and company policies.
|
||
For example, _all tests shall pass on all platforms_ or _at least two people
|
||
shall sign off on code in production_.
|
||
|
||
Many successful software companies, including Google, use code review as a
|
||
standard, integral stage in the software development process.
|
||
|
||
|
||
[[web]]
|
||
== Web-based code review
|
||
|
||
To review work, the Git and Linux Kernel projects send patches via email.
|
||
|
||
Code Review (Gerrit) adds a modern web interface to this workflow. Rather than
|
||
send patches and comments via email, Gerrit users push commits to Gerrit where
|
||
diffs are displayed on a web page. Reviewers can post comments directly on the
|
||
diff. If a change must be reworked, users can push a new, amended revision of
|
||
the same change. Reviewers can then check if the new revision addresses the
|
||
original concerns. If not, the process is repeated.
|
||
|
||
|
||
[[magic]]
|
||
== Gerrit’s magic
|
||
|
||
When you push a change to Gerrit, how does Gerrit detect that the commit amends
|
||
a previous change? Gerrit can’t use the SHA-1, since that value changes when
|
||
`git commit --amend` is called. Fortunately, upon amending a commit, the commit
|
||
message is retained by default.
|
||
|
||
This is where Gerrit's solution lies: Gerrit identifies a conceptual change
|
||
with a footer in the commit message. Each commit message footer contains a
|
||
Change-Id message hook, which uniquely identifies a change across all its
|
||
drafts. For example:
|
||
|
||
`Change-Id: I9e29f5469142cc7fce9e90b0b09f5d2186ff0990`
|
||
|
||
Thus, if the Change-Id remains the same as commits are amended, Gerrit detects
|
||
that each new version refers to the same conceptual change. The Gerrit web
|
||
interface groups versions so that reviewers can see how your change evolves
|
||
during the code review.
|
||
|
||
To Gerrit, the identifier can be random.
|
||
|
||
Gerrit provides a client-side link:cmd-hook-commit-msg.html[message hook] to
|
||
automatically add to commit messages when necessary.
|