Marian Harbach | ebeb154 | 2019-12-13 10:42:46 +0100 | [diff] [blame] | 1 | :linkattrs: |
Janet Davies | 3c58389 | 2018-07-25 13:27:24 -0700 | [diff] [blame] | 2 | = Use Gerrit to Be a Rockstar Programmer |
| 3 | |
| 4 | == Overview |
| 5 | |
| 6 | The term _rockstar_ is often used to describe those talented programmers who |
| 7 | seem to work faster and better than everyone else, much like a composer who |
| 8 | seems to effortlessly churn out fantastic music. However, just as the |
| 9 | spontaneity of masterful music is a fantasy, so is the development of |
| 10 | exceptional code. |
| 11 | |
| 12 | The process of composing and then recording music is painstaking — the artist |
| 13 | records portions of a composition over and over, changing each take until one |
| 14 | song is completed by combining those many takes into a cohesive whole. The end |
| 15 | result is the recording of the best performance of the best version of the |
| 16 | song. |
| 17 | |
| 18 | Consider Queen’s six-minute long Bohemian Rhapsody, which took three weeks to |
| 19 | record. Some segments were overdubbed 180 times! |
| 20 | |
| 21 | Software engineering is much the same. Changes that seem logical and |
| 22 | straightforward in retrospect actually required many revisions and many hours |
| 23 | of work before they were ready to be merged into a code base. A single |
| 24 | conceptual code change (_fix bug 123_) often requires numerous iterations |
| 25 | before it can be finalized. Programmers typically: |
| 26 | |
| 27 | * Fix compilation errors |
| 28 | * Factor out a method, to avoid duplicate code |
| 29 | * Use a better algorithm, to make it faster |
| 30 | * Handle error conditions, to make it more robust |
| 31 | * Add tests, to prevent a bug from regressing |
| 32 | * Adapt tests, to reflect changed behavior |
| 33 | * Polish code, to make it easier to read |
| 34 | * Improve the commit message, to explain why a change was made |
| 35 | |
| 36 | In fact, first drafts of code changes are best kept out of project history. Not |
| 37 | just because rockstar programmers want to hide sloppy first attempts at making |
| 38 | something work. It's more that keeping intermediate states hampers effective |
| 39 | use of version control. Git works best when one commit corresponds to one |
| 40 | functional change, as is required for: |
| 41 | |
| 42 | * git revert |
| 43 | |
| 44 | * git cherry-pick |
| 45 | |
Marian Harbach | 3425337 | 2019-12-10 18:01:31 +0100 | [diff] [blame] | 46 | * link:https://www.kernel.org/pub/software/scm/git/docs/git-bisect-lk2009.html[git bisect,role=external,window=_blank] |
Janet Davies | 3c58389 | 2018-07-25 13:27:24 -0700 | [diff] [blame] | 47 | |
| 48 | |
| 49 | [[amending]] |
| 50 | == Amending commits |
| 51 | |
| 52 | Git provides a mechanism to continually update a commit until it’s perfect: use |
| 53 | `git commit --amend` to remake (re-record) a code change. After you update a |
| 54 | commit in this way, your branch then points to the new commit. However, the |
| 55 | older (imperfect) revision is not lost. It can be found via the `git reflog`. |
| 56 | |
| 57 | |
| 58 | [[review]] |
| 59 | == Code review |
| 60 | |
| 61 | At least two well-known open source projects insist on these practices: |
| 62 | |
Marian Harbach | 3425337 | 2019-12-10 18:01:31 +0100 | [diff] [blame] | 63 | * link:http://git-scm.com/[Git,role=external,window=_blank] |
| 64 | * link:http://www.kernel.org/category/about.html[Linux Kernel,role=external,window=_blank] |
Janet Davies | 3c58389 | 2018-07-25 13:27:24 -0700 | [diff] [blame] | 65 | |
| 66 | However, contributors to these projects don’t refine and polish their changes |
| 67 | in private until they’re perfect. Instead, polishing code is part of a review |
| 68 | process — the contributor offers their change to the project for other |
| 69 | developers to evaluate and critique. This process is called _code review_ and |
| 70 | results in numerous benefits: |
| 71 | |
| 72 | * Code reviews mean that every change effectively has shared authorship |
| 73 | |
| 74 | * Developers share knowledge in two directions: Reviewers learn from the patch |
| 75 | author how the new code they will have to maintain works, and the patch |
| 76 | author learns from reviewers about best practices used in the project. |
| 77 | |
| 78 | * Code review encourages more people to read the code contained in a given |
| 79 | change. As a result, there are more opportunities to find bugs and suggest |
| 80 | improvements. |
| 81 | |
| 82 | * The more people who read the code, the more bugs can be identified. Since |
| 83 | code review occurs before code is submitted, bugs are squashed during the |
| 84 | earliest stage of the software development lifecycle. |
| 85 | |
| 86 | * The review process provides a mechanism to enforce team and company policies. |
| 87 | For example, _all tests shall pass on all platforms_ or _at least two people |
| 88 | shall sign off on code in production_. |
| 89 | |
| 90 | Many successful software companies, including Google, use code review as a |
| 91 | standard, integral stage in the software development process. |
| 92 | |
| 93 | |
| 94 | [[web]] |
| 95 | == Web-based code review |
| 96 | |
| 97 | To review work, the Git and Linux Kernel projects send patches via email. |
| 98 | |
| 99 | Code Review (Gerrit) adds a modern web interface to this workflow. Rather than |
| 100 | send patches and comments via email, Gerrit users push commits to Gerrit where |
| 101 | diffs are displayed on a web page. Reviewers can post comments directly on the |
| 102 | diff. If a change must be reworked, users can push a new, amended revision of |
| 103 | the same change. Reviewers can then check if the new revision addresses the |
| 104 | original concerns. If not, the process is repeated. |
| 105 | |
| 106 | |
| 107 | [[magic]] |
| 108 | == Gerrit’s magic |
| 109 | |
| 110 | When you push a change to Gerrit, how does Gerrit detect that the commit amends |
| 111 | a previous change? Gerrit can’t use the SHA-1, since that value changes when |
| 112 | `git commit --amend` is called. Fortunately, upon amending a commit, the commit |
| 113 | message is retained by default. |
| 114 | |
| 115 | This is where Gerrit's solution lies: Gerrit identifies a conceptual change |
| 116 | with a footer in the commit message. Each commit message footer contains a |
| 117 | Change-Id message hook, which uniquely identifies a change across all its |
| 118 | drafts. For example: |
| 119 | |
| 120 | `Change-Id: I9e29f5469142cc7fce9e90b0b09f5d2186ff0990` |
| 121 | |
| 122 | Thus, if the Change-Id remains the same as commits are amended, Gerrit detects |
| 123 | that each new version refers to the same conceptual change. The Gerrit web |
| 124 | interface groups versions so that reviewers can see how your change evolves |
| 125 | during the code review. |
| 126 | |
| 127 | To Gerrit, the identifier can be random. |
| 128 | |
| 129 | Gerrit provides a client-side link:cmd-hook-commit-msg.html[message hook] to |
| 130 | automatically add to commit messages when necessary. |