Ed Costello | 7407a92 | 2011-07-23 21:38:42 +1200 | [diff] [blame] | 1 | Gerrit Code Review - A Quick Introduction |
| 2 | ========================================= |
| 3 | |
| 4 | Gerrit is a web-based code review tool built on top of the git version |
| 5 | control system, but if you've got as far as reading this guide then |
| 6 | you probably already know that. The purpose of this introduction is to |
| 7 | allow you to answer the question, is Gerrit the right tool for me? |
| 8 | Will it fit in my work flow and in my organization? |
| 9 | |
| 10 | What is Gerrit? |
| 11 | --------------- |
| 12 | |
| 13 | I assume that if you're reading this then you're already convinced of |
| 14 | the benefits of code review in general but want some technical support |
| 15 | to make it easy. Code reviews mean different things to different people. |
| 16 | To some it's a formal meeting with a projector and an entire team |
| 17 | going through the code line by line. To others it's getting someone to |
| 18 | glance over the code before it is committed. |
| 19 | |
| 20 | Gerrit is intended to provide a light weight framework for reviewing |
| 21 | every commit before it is accepted into the code base. Changes are |
| 22 | uploaded to Gerrit but don't actually become a part of the project |
| 23 | until they've been reviewed and accepted. In many ways this is simply |
| 24 | tooling to support the standard open source process of submitting |
| 25 | patches which are then reviewed by the project members before being |
| 26 | applied to the code base. However Gerrit goes a step further making it |
| 27 | simple for all committers on a project to ensure that changes are |
| 28 | checked over before they're actually applied. Because of this Gerrit |
| 29 | is equally useful where all users are trusted committers such as may |
David Pursehouse | 221d4f6 | 2012-06-08 17:38:08 +0900 | [diff] [blame] | 30 | be the case with closed-source commercial development. Either way it's |
Ed Costello | 7407a92 | 2011-07-23 21:38:42 +1200 | [diff] [blame] | 31 | still desirable to have code reviewed to improve the quality and |
| 32 | maintainability of the code. After all, if only one person has seen |
| 33 | the code it may be a little difficult to maintain when that person |
| 34 | leaves. |
| 35 | |
| 36 | Gerrit is firstly a staging area where changes can be checked over |
| 37 | before becoming a part of the code base. It is also an enabler for |
| 38 | this review process, capturing notes and comments about the changes to |
| 39 | enable discussion of the change. This is particularly useful with |
| 40 | distributed teams where this conversation can't happen face to face. |
| 41 | Even with a co-located team having a review tool as an option is |
| 42 | beneficial because reviews can be done at a time that is convenient |
| 43 | for the reviewer. This allows the developer to create the review and |
| 44 | explain the change while it is fresh in their mind. Without such a |
| 45 | tool they either need to interrupt someone to review the code or |
| 46 | switch context to explain the change when they've already moved on to |
| 47 | the next task. |
| 48 | |
| 49 | This also creates a lasting record of the conversation which can be |
| 50 | useful for answering the inevitable "I know we changed this for a |
| 51 | reason" questions. |
| 52 | |
| 53 | Where does Gerrit fit in? |
| 54 | ------------------------- |
| 55 | |
| 56 | Any team with more than one member has a central source repository of |
| 57 | some kind (or they should). Git can theoretically work without such a |
| 58 | central location but in practice there is usually a central |
| 59 | repository. This serves as the authoritative copy of what is actually in |
| 60 | the project. This is what everyone fetches from and pushes to and is |
| 61 | generally where build servers and other such tools get the source |
| 62 | from. |
| 63 | |
| 64 | .Central Source Repository |
| 65 | image::images/intro-quick-central-repo.png[Authoritative Source Repository] |
| 66 | |
| 67 | Gerrit is deployed in place of this central repository and adds an |
| 68 | additional concept, a store of pending changes. Everyone still fetches |
| 69 | from the authoritative repository but instead of pushing back to it, |
| 70 | they push to this pending changes location. A change can only be submitted |
| 71 | into the authoritative repository and become an accepted part of the project |
| 72 | once the change has been reviewed and approved. |
| 73 | |
| 74 | .Gerrit in place of Central Repository |
| 75 | image::images/intro-quick-central-gerrit.png[Gerrit in place of Central Repository] |
| 76 | |
| 77 | Like any repository hosting solution, Gerrit has a powerful |
| 78 | link:access-control.html[access control model.] |
| 79 | Users can even be granted access to push directly into the central |
| 80 | repository, bypassing code review entirely. Gerrit can even be used |
| 81 | without code review, used simply to host the repositories and |
| 82 | controlling access. But generally it's just simpler and safer to go |
| 83 | through the review process even for users who are allowed to directly |
| 84 | push. |
| 85 | |
| 86 | The Life and Times of a Change |
| 87 | ------------------------------ |
| 88 | |
| 89 | The easiest way to get a feel for how Gerrit works is to follow a |
| 90 | change through its entire life cycle. For the purpose of this example |
| 91 | we'll assume that the Gerrit Server is running on a server called |
| 92 | +gerrithost+ with the HTTP interface on port +8080+ and the SSH |
| 93 | interface on port +29418+. The project we'll be working on is called |
| 94 | +RecipeBook+ and we'll be developing a change for the +master+ branch. |
| 95 | |
| 96 | Cloning the Repository |
| 97 | ~~~~~~~~~~~~~~~~~~~~~~ |
| 98 | |
| 99 | Obviously the first thing we need to do is get the source that we're |
| 100 | going to be modifying. As with any git project you do this by cloning |
| 101 | the central repository that Gerrit is hosting. e.g. |
| 102 | |
| 103 | ---- |
| 104 | $ git clone ssh://gerrithost:29418/RecipeBook.git RecipeBook |
| 105 | Cloning into RecipeBook... |
| 106 | ---- |
| 107 | |
| 108 | Then we need to make our actual change and commit it locally. Gerrit |
| 109 | doesn't really change anything here, this is just the standard editing |
| 110 | and git. While not strictly required, it's best to include a Change-Id |
| 111 | in your commit message so that Gerrit can link together different |
| 112 | versions of the same change being reviewed. Gerrit contains a standard |
| 113 | link:user-changeid.html[Change-Id commit-msg hook] |
| 114 | that will generate a unique Change-Id when you commit. If you don't do |
| 115 | this then Gerrit will generate a Change-Id when you push your change |
| 116 | for review. But because you don't have the Change-Id in your commit |
| 117 | message you'll need to manually copy it in if you need to upload |
| 118 | another version of your change. Because of this it's best to just |
| 119 | install the hook and forget about it. |
| 120 | |
| 121 | Creating the Review |
| 122 | ~~~~~~~~~~~~~~~~~~~ |
| 123 | |
| 124 | Once you've made your change and committed it locally it's time to |
| 125 | push it to Gerrit so that it can be reviewed. This is done with a git |
| 126 | push to the Gerrit server. Since we cloned our local repository |
| 127 | directly from Gerrit it is the origin so we don't have to redefine the |
| 128 | remote. |
| 129 | |
| 130 | ---- |
| 131 | $ <work> |
| 132 | $ git commit |
| 133 | [master 9651f22] Change to a proper, yeast based pizza dough. |
| 134 | 1 files changed, 3 insertions(+), 2 deletions(-) |
| 135 | $ git push origin HEAD:refs/for/master |
| 136 | Counting objects: 5, done. |
| 137 | Delta compression using up to 8 threads. |
| 138 | Compressing objects: 100% (2/2), done. |
| 139 | Writing objects: 100% (3/3), 542 bytes, done. |
| 140 | Total 3 (delta 0), reused 0 (delta 0) |
Robin Rosenberg | 524a303 | 2012-10-14 14:24:36 +0200 | [diff] [blame] | 141 | remote: |
Ed Costello | 7407a92 | 2011-07-23 21:38:42 +1200 | [diff] [blame] | 142 | remote: New Changes: |
| 143 | remote: http://gerrithost:8080/68 |
Robin Rosenberg | 524a303 | 2012-10-14 14:24:36 +0200 | [diff] [blame] | 144 | remote: |
Ed Costello | 7407a92 | 2011-07-23 21:38:42 +1200 | [diff] [blame] | 145 | To ssh://gerrithost:29418/RecipeBook.git |
| 146 | * [new branch] HEAD -> refs/for/master |
| 147 | ---- |
| 148 | |
| 149 | The only different thing about this is the +refs/for/master+ branch. |
| 150 | This is a magic branch that creates reviews that target the master |
| 151 | branch. For every branch Gerrit tracks there is a magic |
| 152 | +refs/for/<branch_name>+ that you push to to create reviews. |
| 153 | |
| 154 | In the output of this command you'll notice that there is a link to |
| 155 | the HTTP interface of the Gerrit server we just pushed to. This is the |
| 156 | web interface where we will review this commit. Let's follow that link |
| 157 | and see what we get. |
| 158 | |
| 159 | .Gerrit Code Review Screen |
| 160 | image::images/intro-quick-new-review.jpg[Gerrit Review Screen] |
| 161 | |
| 162 | This is the Gerrit code review screen where someone will come to |
| 163 | review the change. There isn't too much to see here yet, you can look |
| 164 | at the diff of your change, add some comments explaining what you did |
| 165 | and why, you may even add a list of people that should review the change. |
| 166 | |
| 167 | Reviewers can find changes that they want to review in any number of |
| 168 | ways. Gerrit has a capable |
| 169 | link:user-search.html[search] |
| 170 | that allows project leaders (or anyone else) to find changes that need |
| 171 | to be reviewed. Users can also setup watches on Gerrit projects with a |
| 172 | search expression, this causes Gerrit to notify them of matching |
| 173 | changes. So adding a reviewer when creating a review is just a |
| 174 | recommendation. |
| 175 | |
| 176 | At this point the change is available for review and we need to switch |
| 177 | roles to continue following the change. Now let's pretend we're the |
| 178 | reviewer. |
| 179 | |
| 180 | Reviewing the Change |
| 181 | ~~~~~~~~~~~~~~~~~~~~ |
| 182 | |
| 183 | The reviewer's life starts at the code review screen shown above. He |
| 184 | can get here in a number of ways, but for some reason they've decided |
| 185 | to review this change. Of particular note on this screen are the two |
| 186 | "Need" lines: |
| 187 | |
| 188 | ---- |
| 189 | * Need Verified |
| 190 | * Need Code-Review |
| 191 | ---- |
| 192 | |
| 193 | Gerrit's default work-flow requires two checks before a change is |
| 194 | accepted. Code-Review is someone looking at the code, ensuring it |
| 195 | meets the project guidelines, intent etc. Verifying is checking that |
| 196 | the code actually compiles, unit tests pass etc. Verification is |
| 197 | usually done by an automated build server rather than a person. There |
| 198 | is even a |
| 199 | link:https://wiki.jenkins-ci.org/display/JENKINS/Gerrit+Trigger[Gerrit Trigger Jenkins Plugin] |
| 200 | that will automatically build each uploaded change and update the |
| 201 | verified score accordingly. |
| 202 | |
| 203 | It is important to note that Code-Review and Verification are |
| 204 | different permissions in Gerrit, allowing these tasks to be separated. |
| 205 | For example, an automated process would have rights to verify but not |
| 206 | to code-review. |
| 207 | |
| 208 | Since we are the code reviewer, we're going to review the code. To do |
| 209 | this we can view it within the Gerrit web interface as either a |
| 210 | unified or side-by-side diff by selecting the appropriate option. In |
| 211 | the example below we've selected the side-by-side view. In either of |
Bruce Zu | 6b0fd76 | 2012-10-25 16:52:00 +0800 | [diff] [blame] | 212 | these views you can add inline comments by double clicking on the line |
| 213 | (or single click the line number) that you want to comment on. Also you |
| 214 | can add file comment by double clicking anywhere (not just on the |
| 215 | "Patch Set" words) in the table header or single clicking on the icon |
| 216 | in the line-number column header. Once published these comments are |
| 217 | viewable to all, allowing discussion of the change to take place. |
Ed Costello | 7407a92 | 2011-07-23 21:38:42 +1200 | [diff] [blame] | 218 | |
| 219 | .Side By Side Patch View |
| 220 | image::images/intro-quick-review-line-comment.jpg[Adding a Comment] |
| 221 | |
| 222 | Code reviewers end up spending a lot of time navigating these screens, |
| 223 | looking at and commenting on these changes. To make this as efficient |
| 224 | as possible Gerrit has keyboard shortcuts for most operations (and |
| 225 | even some operations that are only accessible via the hot-keys). At |
| 226 | any time you can hit the +?+ key to see the keyboard shortcuts. |
| 227 | |
| 228 | .Gerrit Hot Key Help |
| 229 | image::images/intro-quick-hot-key-help.jpg[Hot Key Help] |
| 230 | |
| 231 | Once we've looked over the changes we need to complete reviewing the |
| 232 | submission. To do this we click the _Review_ button on the change |
| 233 | screen where we started. This allows us to enter a Code Review label |
| 234 | and message. |
| 235 | |
| 236 | .Reviewing the Change |
| 237 | image::images/intro-quick-reviewing-the-change.jpg[Reviewing the Change] |
| 238 | |
| 239 | The label that the reviewer selects determines what can happen next. |
| 240 | The +1 and -1 level are just an opinion where as the +2 and -2 levels |
| 241 | are allowing or blocking the change. In order for a change to be |
| 242 | accepted it must have at least one +2 and no -2 votes. |
| 243 | Although these are numeric values, they in no way accumulate; |
| 244 | two +1s do not equate to a +2. |
| 245 | |
| 246 | Regardless of what label is selected, once the _Publish Comments_ |
| 247 | button has been clicked, the cover message and any comments on the |
| 248 | files become visible to all users. |
| 249 | |
| 250 | In this case the change was not accepted so the creator needs to |
| 251 | rework it. So let's switch roles back to the creator where we |
| 252 | started. |
| 253 | |
| 254 | Reworking the Change |
| 255 | ~~~~~~~~~~~~~~~~~~~~ |
| 256 | |
| 257 | As long as we set up the |
| 258 | link:user-changeid.html[Change-Id commit-msg hook] |
| 259 | before we uploaded the change, re-working it is easy. All we need |
| 260 | to do to upload a re-worked change is to push another commit that has |
| 261 | the same Change-Id in the message. Since the hook added a Change-ID in |
| 262 | our initial commit we can simply checkout and then amend that commit. |
| 263 | Then push it to Gerrit in the same way as we did to create the review. E.g. |
| 264 | |
| 265 | ---- |
| 266 | $ <checkout first commit> |
| 267 | $ <rework> |
| 268 | $ git commit --amend |
| 269 | $ git push origin HEAD:refs/for/master |
| 270 | Counting objects: 5, done. |
| 271 | Delta compression using up to 8 threads. |
| 272 | Compressing objects: 100% (2/2), done. |
| 273 | Writing objects: 100% (3/3), 546 bytes, done. |
| 274 | Total 3 (delta 0), reused 0 (delta 0) |
| 275 | To ssh://gerrithost:29418/RecipeBook.git |
| 276 | * [new branch] HEAD -> refs/for/master |
| 277 | ---- |
| 278 | |
| 279 | Note that the output is slightly different this time around. We don't |
| 280 | get told about a new review because we're adding to an existing |
| 281 | review. Having uploaded the reworked commit we can go back into the |
| 282 | Gerrit web interface and look at our change. |
| 283 | |
| 284 | .Reviewing the Rework |
| 285 | image::images/intro-quick-review-2-patches.jpg[Reviewing the Rework] |
| 286 | |
| 287 | If you look closely you'll notice that there are now two patch sets |
| 288 | associated with this change, the initial submission and the rework. |
| 289 | Rather than repeating ourselves lets assume that this time around the |
| 290 | patch is given a +2 score by the code reviewer. |
| 291 | |
| 292 | Trying out the Change |
| 293 | ~~~~~~~~~~~~~~~~~~~~~ |
| 294 | |
| 295 | With Gerrit's default work-flow there are two sign-offs, code review |
| 296 | and verify. Verifying means checking that the change actually works. |
| 297 | This would typically be checking that the code compiles, unit tests |
| 298 | pass and similar checks. Really a project can decide how much or |
| 299 | little they want to do here. It's also worth noting that this is only |
| 300 | Gerrit's default work-flow, the verify check can actually be removed |
| 301 | or others added. |
| 302 | |
| 303 | As mentioned in the code review section, verification is typically an |
| 304 | automated process using the |
| 305 | link:https://wiki.jenkins-ci.org/display/JENKINS/Gerrit+Trigger[Gerrit Trigger Jenkins Plugin] |
| 306 | or similar. But there are times when the code needs to be manually |
| 307 | verified, or the reviewer needs to check that something actually works |
| 308 | or how it works. Sometimes it's just nice to work through the code in a |
| 309 | development environment rather than the web interface. All of these |
| 310 | involve someone needing to get the change into their development |
| 311 | environment. Gerrit makes this process easy by exposing each change as |
| 312 | a git branch. So all the reviewers need to do is fetch and checkout that |
| 313 | branch from Gerrit and they will have the change. |
| 314 | |
| 315 | We don't even need to think about it that hard, if you look at the |
| 316 | earlier screen shots of the Gerrit Code Review Screen you'll notice a |
| 317 | _download_ command. All we need to do to get the change is copy |
| 318 | paste this command and run it in our Gerrit checkout. |
| 319 | |
| 320 | ---- |
| 321 | $ git fetch http://gerrithost:8080/p/RecipeBook refs/changes/68/68/2 |
| 322 | From http://gerrithost:8080/p/RecipeBook |
| 323 | * branch refs/changes/68/68/2 -> FETCH_HEAD |
| 324 | $ git checkout FETCH_HEAD |
| 325 | Note: checking out 'FETCH_HEAD'. |
| 326 | |
| 327 | You are in 'detached HEAD' state. You can look around, make experimental |
| 328 | changes and commit them, and you can discard any commits you make in this |
| 329 | state without impacting any branches by performing another checkout. |
| 330 | |
| 331 | If you want to create a new branch to retain commits you create, you may |
| 332 | do so (now or later) by using -b with the checkout command again. Example: |
| 333 | |
| 334 | git checkout -b new_branch_name |
| 335 | |
| 336 | HEAD is now at d5dacdb... Change to a proper, yeast based pizza dough. |
| 337 | ---- |
| 338 | |
| 339 | Easy as that, we now have the change in our working copy to play with. |
| 340 | You might be interested in what the numbers of the refspec mean. |
| 341 | |
David Pursehouse | 221d4f6 | 2012-06-08 17:38:08 +0900 | [diff] [blame] | 342 | * The first *68* is the id of the change +mod 100+. The only reason |
Ed Costello | 7407a92 | 2011-07-23 21:38:42 +1200 | [diff] [blame] | 343 | for this initial number is to reduce the number of files in any given |
| 344 | directory within the git repository. |
| 345 | * The second *68* is the full id of the change. You'll notice this in |
| 346 | the URL of the Gerrit review screen. |
| 347 | * The *2* is the patch-set within the change. In this example we |
| 348 | uploaded some fixes so we want the second patch set rather than the |
| 349 | initial one which the reviewer rejected. |
| 350 | |
| 351 | Manually Verifying the Change |
| 352 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
| 353 | |
| 354 | For simplicity we're just going to manually verify the change. |
| 355 | The Verifier may be the same person as the code reviewer or a |
| 356 | different person entirely. It really depends on the size of the |
| 357 | project and what works. If you have Verify permission then when you |
| 358 | click the _Review_ button in the Gerrit web interface you'll be |
| 359 | presented with a verify score. |
| 360 | |
| 361 | .Verifying the Change |
| 362 | image::images/intro-quick-verifying.jpg[Verifying the Change] |
| 363 | |
| 364 | Unlike the code review the verify check doesn't have a +2 or -2 level, |
| 365 | it's either a pass or fail so all we need for the change to be |
| 366 | submitted is a +1 score (and no -1's). |
| 367 | |
| 368 | Submitting the Change |
| 369 | ~~~~~~~~~~~~~~~~~~~~~ |
| 370 | |
| 371 | You might have noticed that in the verify screen shot there are two |
| 372 | buttons for submitting the score _Publish Comments_ and _Publish |
| 373 | and Submit_. The publish and submit button is always visible, but will |
| 374 | only work if the change meets the criteria for being submitted (I.e. |
| 375 | has been both verified and code reviewed). So it's a convenience to be |
| 376 | able to post review scores as well as submitting the change by clicking |
| 377 | a single button. If you choose just to publish comments at this point then |
| 378 | the score will be stored but the change won't yet be accepted into the code |
| 379 | base. In this case there will be a _Submit Patch Set X_ button on the |
Dave Borowitz | 01c1b1f | 2013-02-27 13:49:04 -0800 | [diff] [blame] | 380 | main screen. Just as Code-Review and Verify are different operations |
Ed Costello | 7407a92 | 2011-07-23 21:38:42 +1200 | [diff] [blame] | 381 | that can be done by different users, Submission is a third operation |
| 382 | that can be limited down to another group of users. |
| 383 | |
David Pursehouse | 221d4f6 | 2012-06-08 17:38:08 +0900 | [diff] [blame] | 384 | Clicking the _Publish and Submit_ or _Submit Patch Set X_ button |
Ed Costello | 7407a92 | 2011-07-23 21:38:42 +1200 | [diff] [blame] | 385 | will merge the change into the main part of the repository so that it |
| 386 | becomes an accepted part of the project. After this anyone fetching |
| 387 | the git repository will receive this change as a part of the master |
| 388 | branch. |
| 389 | |
| 390 | GERRIT |
| 391 | ------ |
| 392 | Part of link:index.html[Gerrit Code Review] |