From 3c5838908327f3f8f1601316e567264bccbd5fa3 Mon Sep 17 00:00:00 2001 From: Janet Davies Date: Wed, 25 Jul 2018 13:27:24 -0700 Subject: [PATCH] Add intro to documentation to describe the rationale of Gerrit code review. Based on a draft provided by Han-Wen Nienhuys (hanwen@google.com) Change-Id: Ib593bf6a5c78b9b5c44b062a205eb03fef926b19 --- Documentation/index.txt | 1 + Documentation/intro-rockstar.txt | 129 +++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+) create mode 100644 Documentation/intro-rockstar.txt diff --git a/Documentation/index.txt b/Documentation/index.txt index 51ce9d61bc..84925f4f98 100644 --- a/Documentation/index.txt +++ b/Documentation/index.txt @@ -5,6 +5,7 @@ . link:linux-quickstart.html[Quickstart for Installing Gerrit on Linux] == About Gerrit +. link:intro-rockstar.html[Why Code Review?] . link:intro-quick.html[Product Overview] . link:intro-how-gerrit-works.html[How Gerrit Works] . link:intro-gerrit-walkthrough.html[Basic Gerrit Walkthrough] diff --git a/Documentation/intro-rockstar.txt b/Documentation/intro-rockstar.txt new file mode 100644 index 0000000000..b60a91fc73 --- /dev/null +++ b/Documentation/intro-rockstar.txt @@ -0,0 +1,129 @@ += 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.