blob: 24c8753a5e45feb223c68918c5e4b46db2b58283 [file] [log] [blame] [view]
Mathieu Perreault339625382017-07-29 00:32:581# Respectful Changes
2## A Guide for Code Authors
3
4_For the code reviewer counterpart, see
5__[Respectful Code Reviews](cr_respect.md)__._
6
7## Set up for success
8
9#### Do the pre-work
10
11Help challenging code reviews go smoothly by reaching out to prospective
12reviewers before writing any code. Describing the problem and your approach
13ahead of time reduces surprise and provides an opportunity for early input.
14Ensure the decisions resulting from these exchanges, as well as the reasoning
15behind them, are accessible to others (e.g. via bug or design doc).
16
17#### Mind your reviewer
18
19Make choices that spare your reviewer time or cognitive load, such as preferring
20a series of short changes to a massive one, or uploading separate patches to
21isolate rebases during review.
22
23#### Satisfy preconditions
24
25Ensure your code is ready for review before you send it: it should compile, have
26adequate testing that passes, and respect the style guide (using _git cl
27format/lint_ is encouraged). Consider validating this by performing a
28self-review. This is respectful of reviewer time and can sometimes save you a
29review round trip. If you're looking for an early review that's fine too, but
30please say so.
31
32#### Remember communication can be hard
33
34Differences in understanding or opinions are to be expected in the context of
35code reviews. Always assume competence and goodwill. Don't hesitate to suggest a
36quick meeting (face-to-face or via VC); sometimes it's much faster to resolve an
37issue that way than email ping pong.
38
39## Request the review
40
41#### Choose your reviewers
42
43Give thought to whether you want to serialize or parallelize your reviews. If
44you're new to the codebase, it's a good idea to do a first round with a single
45local reviewer to clear the basic issues. Try to limit the number of owners you
46solicit (only one per section), but ensure you pick sufficiently specialized
47ones. Finally, be mindful of time zones and their effect on the review cycle
Evan Stadece7bcd12025-03-31 16:46:4648time. Picking the right reviewers comes with experience, but start
49[here](cl_tips.md#choosing-the-right-reviewers).
Mathieu Perreault339625382017-07-29 00:32:5850
51#### Provide context
52
53Change descriptions are the first impression your change makes, both on
Johann48fbca92021-07-16 18:29:4154reviewers and on code archeologists from the future. A [good description](contributing.md#Uploading-a-change-for-review)
Mathieu Perreault339625382017-07-29 00:32:5855aims to do two things. First, it conveys at a glance the high level view.
56Second, it provides references to all the relevant information for a deep dive:
57design docs, bugs, testing instructions. The bug\# is a useful reference, but
58isn't sufficient on its own. Summarize **what** and **why** in the description.
59You can additionally provide guidance on how to do the review in the e-mailed
60message.
61
62#### State your expectations
63
64When sending the review, be clear to your reviewer about your expectations. In
65terms of the review, this means specifying the kind of reviewing (e.g., high
66level) as well as who should review what using which level of scrutiny. In terms
67of timing, this means stating your deadline or lack thereof. For tight
68deadlines, be convinced your urgency is real (hint: should be rare), and
69communicate its reason, as well as your intent to land required follow up
70refactorings.
71
72## During the review
73
74#### Expect responsiveness
75
76Getting your code reviewed is about getting unblocked. You should expect
77reviewer input within 1 business day. This should however be modulated based on
78the size, complexity, urgency / importance of your change, as well as on
79time zone differences. Beyond that, double check the reviewer's code review tool
80nickname (e.g. "_jdoe (OOO til 4 Apr)_"), their calendar and ping them on IM. If
81that fails, look for another reviewer.
82
83#### Address all comments
84
85Be convinced your reviewers feel all comments have been addressed before you
86commit. Questions are addressed by providing an answer. Suggestions can be
87addressed in one of three ways: adopt it immediately ("Done."), defer it to a
88subsequent change (TODO with a bug \#) or push back with additional
89information. Whenever more information is required, make sure everyone agrees on
90the problem before you discuss the solution and consider expanding the
91documentation.
92
kalettuceaebff762023-04-06 20:43:0793#### Wait for LGTM from all your reviewers
94
95As a general rule of thumb, if a reviewer has made a comment on your CL, even
96though you may have addressed that comment in a new patchset, don't submit the
97CL until you have their LGTM, unless the reviewer gave the OK to do so (e.g.
98when the reviewer delegates the reviewing task to someone else). If you need to
99land a CL urgently and one of your reviewers isn't available (e.g. OOO), submit
kalettucedd7e88462023-04-06 23:02:55100your CL, and send your reviewer a note; in the note, be sure to include the
101reason why you had to land the CL, and show that you've considered their
102opinions & are ready to promptly act on their additional comments in a followup
103CL.
kalettuceaebff762023-04-06 20:43:07104
Mathieu Perreault339625382017-07-29 00:32:58105#### What to do if it's going wrong
106
107Code reviews should not make you feel bad. If you find yourself in that
108situation, or you feel the review's at an impasse, don't attempt to work around
109a reviewer but take a step back. A face to face meeting or a VC can sometimes
110help unblock a review. If this doesn't sound like an option, or simply if you
111feel you need to talk about it, reach out to someone you trust.
112
113## After the review
114
115Code reviews are in large part about having others watch your back. Don't
116hesitate to say "Thank you" once the review is completed. Additionally, if
117you're new to code reviews, take a few moments to reflect on what went well or
118didn't.