diff options
Diffstat (limited to 'Documentation/development-process/6.Followthrough')
-rw-r--r-- | Documentation/development-process/6.Followthrough | 202 |
1 files changed, 202 insertions, 0 deletions
diff --git a/Documentation/development-process/6.Followthrough b/Documentation/development-process/6.Followthrough new file mode 100644 index 00000000000..a8fba3d83a8 --- /dev/null +++ b/Documentation/development-process/6.Followthrough | |||
@@ -0,0 +1,202 @@ | |||
1 | 6: FOLLOWTHROUGH | ||
2 | |||
3 | At this point, you have followed the guidelines given so far and, with the | ||
4 | addition of your own engineering skills, have posted a perfect series of | ||
5 | patches. One of the biggest mistakes that even experienced kernel | ||
6 | developers can make is to conclude that their work is now done. In truth, | ||
7 | posting patches indicates a transition into the next stage of the process, | ||
8 | with, possibly, quite a bit of work yet to be done. | ||
9 | |||
10 | It is a rare patch which is so good at its first posting that there is no | ||
11 | room for improvement. The kernel development process recognizes this fact, | ||
12 | and, as a result, is heavily oriented toward the improvement of posted | ||
13 | code. You, as the author of that code, will be expected to work with the | ||
14 | kernel community to ensure that your code is up to the kernel's quality | ||
15 | standards. A failure to participate in this process is quite likely to | ||
16 | prevent the inclusion of your patches into the mainline. | ||
17 | |||
18 | |||
19 | 6.1: WORKING WITH REVIEWERS | ||
20 | |||
21 | A patch of any significance will result in a number of comments from other | ||
22 | developers as they review the code. Working with reviewers can be, for | ||
23 | many developers, the most intimidating part of the kernel development | ||
24 | process. Life can be made much easier, though, if you keep a few things in | ||
25 | mind: | ||
26 | |||
27 | - If you have explained your patch well, reviewers will understand its | ||
28 | value and why you went to the trouble of writing it. But that value | ||
29 | will not keep them from asking a fundamental question: what will it be | ||
30 | like to maintain a kernel with this code in it five or ten years later? | ||
31 | Many of the changes you may be asked to make - from coding style tweaks | ||
32 | to substantial rewrites - come from the understanding that Linux will | ||
33 | still be around and under development a decade from now. | ||
34 | |||
35 | - Code review is hard work, and it is a relatively thankless occupation; | ||
36 | people remember who wrote kernel code, but there is little lasting fame | ||
37 | for those who reviewed it. So reviewers can get grumpy, especially when | ||
38 | they see the same mistakes being made over and over again. If you get a | ||
39 | review which seems angry, insulting, or outright offensive, resist the | ||
40 | impulse to respond in kind. Code review is about the code, not about | ||
41 | the people, and code reviewers are not attacking you personally. | ||
42 | |||
43 | - Similarly, code reviewers are not trying to promote their employers' | ||
44 | agendas at the expense of your own. Kernel developers often expect to | ||
45 | be working on the kernel years from now, but they understand that their | ||
46 | employer could change. They truly are, almost without exception, | ||
47 | working toward the creation of the best kernel they can; they are not | ||
48 | trying to create discomfort for their employers' competitors. | ||
49 | |||
50 | What all of this comes down to is that, when reviewers send you comments, | ||
51 | you need to pay attention to the technical observations that they are | ||
52 | making. Do not let their form of expression or your own pride keep that | ||
53 | from happening. When you get review comments on a patch, take the time to | ||
54 | understand what the reviewer is trying to say. If possible, fix the things | ||
55 | that the reviewer is asking you to fix. And respond back to the reviewer: | ||
56 | thank them, and describe how you will answer their questions. | ||
57 | |||
58 | Note that you do not have to agree with every change suggested by | ||
59 | reviewers. If you believe that the reviewer has misunderstood your code, | ||
60 | explain what is really going on. If you have a technical objection to a | ||
61 | suggested change, describe it and justify your solution to the problem. If | ||
62 | your explanations make sense, the reviewer will accept them. Should your | ||
63 | explanation not prove persuasive, though, especially if others start to | ||
64 | agree with the reviewer, take some time to think things over again. It can | ||
65 | be easy to become blinded by your own solution to a problem to the point | ||
66 | that you don't realize that something is fundamentally wrong or, perhaps, | ||
67 | you're not even solving the right problem. | ||
68 | |||
69 | One fatal mistake is to ignore review comments in the hope that they will | ||
70 | go away. They will not go away. If you repost code without having | ||
71 | responded to the comments you got the time before, you're likely to find | ||
72 | that your patches go nowhere. | ||
73 | |||
74 | Speaking of reposting code: please bear in mind that reviewers are not | ||
75 | going to remember all the details of the code you posted the last time | ||
76 | around. So it is always a good idea to remind reviewers of previously | ||
77 | raised issues and how you dealt with them; the patch changelog is a good | ||
78 | place for this kind of information. Reviewers should not have to search | ||
79 | through list archives to familiarize themselves with what was said last | ||
80 | time; if you help them get a running start, they will be in a better mood | ||
81 | when they revisit your code. | ||
82 | |||
83 | What if you've tried to do everything right and things still aren't going | ||
84 | anywhere? Most technical disagreements can be resolved through discussion, | ||
85 | but there are times when somebody simply has to make a decision. If you | ||
86 | honestly believe that this decision is going against you wrongly, you can | ||
87 | always try appealing to a higher power. As of this writing, that higher | ||
88 | power tends to be Andrew Morton. Andrew has a great deal of respect in the | ||
89 | kernel development community; he can often unjam a situation which seems to | ||
90 | be hopelessly blocked. Appealing to Andrew should not be done lightly, | ||
91 | though, and not before all other alternatives have been explored. And bear | ||
92 | in mind, of course, that he may not agree with you either. | ||
93 | |||
94 | |||
95 | 6.2: WHAT HAPPENS NEXT | ||
96 | |||
97 | If a patch is considered to be a good thing to add to the kernel, and once | ||
98 | most of the review issues have been resolved, the next step is usually | ||
99 | entry into a subsystem maintainer's tree. How that works varies from one | ||
100 | subsystem to the next; each maintainer has his or her own way of doing | ||
101 | things. In particular, there may be more than one tree - one, perhaps, | ||
102 | dedicated to patches planned for the next merge window, and another for | ||
103 | longer-term work. | ||
104 | |||
105 | For patches applying to areas for which there is no obvious subsystem tree | ||
106 | (memory management patches, for example), the default tree often ends up | ||
107 | being -mm. Patches which affect multiple subsystems can also end up going | ||
108 | through the -mm tree. | ||
109 | |||
110 | Inclusion into a subsystem tree can bring a higher level of visibility to a | ||
111 | patch. Now other developers working with that tree will get the patch by | ||
112 | default. Subsystem trees typically feed into -mm and linux-next as well, | ||
113 | making their contents visible to the development community as a whole. At | ||
114 | this point, there's a good chance that you will get more comments from a | ||
115 | new set of reviewers; these comments need to be answered as in the previous | ||
116 | round. | ||
117 | |||
118 | What may also happen at this point, depending on the nature of your patch, | ||
119 | is that conflicts with work being done by others turn up. In the worst | ||
120 | case, heavy patch conflicts can result in some work being put on the back | ||
121 | burner so that the remaining patches can be worked into shape and merged. | ||
122 | Other times, conflict resolution will involve working with the other | ||
123 | developers and, possibly, moving some patches between trees to ensure that | ||
124 | everything applies cleanly. This work can be a pain, but count your | ||
125 | blessings: before the advent of the linux-next tree, these conflicts often | ||
126 | only turned up during the merge window and had to be addressed in a hurry. | ||
127 | Now they can be resolved at leisure, before the merge window opens. | ||
128 | |||
129 | Some day, if all goes well, you'll log on and see that your patch has been | ||
130 | merged into the mainline kernel. Congratulations! Once the celebration is | ||
131 | complete (and you have added yourself to the MAINTAINERS file), though, it | ||
132 | is worth remembering an important little fact: the job still is not done. | ||
133 | Merging into the mainline brings its own challenges. | ||
134 | |||
135 | To begin with, the visibility of your patch has increased yet again. There | ||
136 | may be a new round of comments from developers who had not been aware of | ||
137 | the patch before. It may be tempting to ignore them, since there is no | ||
138 | longer any question of your code being merged. Resist that temptation, | ||
139 | though; you still need to be responsive to developers who have questions or | ||
140 | suggestions. | ||
141 | |||
142 | More importantly, though: inclusion into the mainline puts your code into | ||
143 | the hands of a much larger group of testers. Even if you have contributed | ||
144 | a driver for hardware which is not yet available, you will be surprised by | ||
145 | how many people will build your code into their kernels. And, of course, | ||
146 | where there are testers, there will be bug reports. | ||
147 | |||
148 | The worst sort of bug reports are regressions. If your patch causes a | ||
149 | regression, you'll find an uncomfortable number of eyes upon you; | ||
150 | regressions need to be fixed as soon as possible. If you are unwilling or | ||
151 | unable to fix the regression (and nobody else does it for you), your patch | ||
152 | will almost certainly be removed during the stabilization period. Beyond | ||
153 | negating all of the work you have done to get your patch into the mainline, | ||
154 | having a patch pulled as the result of a failure to fix a regression could | ||
155 | well make it harder for you to get work merged in the future. | ||
156 | |||
157 | After any regressions have been dealt with, there may be other, ordinary | ||
158 | bugs to deal with. The stabilization period is your best opportunity to | ||
159 | fix these bugs and ensure that your code's debut in a mainline kernel | ||
160 | release is as solid as possible. So, please, answer bug reports, and fix | ||
161 | the problems if at all possible. That's what the stabilization period is | ||
162 | for; you can start creating cool new patches once any problems with the old | ||
163 | ones have been taken care of. | ||
164 | |||
165 | And don't forget that there are other milestones which may also create bug | ||
166 | reports: the next mainline stable release, when prominent distributors pick | ||
167 | up a version of the kernel containing your patch, etc. Continuing to | ||
168 | respond to these reports is a matter of basic pride in your work. If that | ||
169 | is insufficient motivation, though, it's also worth considering that the | ||
170 | development community remembers developers who lose interest in their code | ||
171 | after it's merged. The next time you post a patch, they will be evaluating | ||
172 | it with the assumption that you will not be around to maintain it | ||
173 | afterward. | ||
174 | |||
175 | |||
176 | 6.3: OTHER THINGS THAT CAN HAPPEN | ||
177 | |||
178 | One day, you may open your mail client and see that somebody has mailed you | ||
179 | a patch to your code. That is one of the advantages of having your code | ||
180 | out there in the open, after all. If you agree with the patch, you can | ||
181 | either forward it on to the subsystem maintainer (be sure to include a | ||
182 | proper From: line so that the attribution is correct, and add a signoff of | ||
183 | your own), or send an Acked-by: response back and let the original poster | ||
184 | send it upward. | ||
185 | |||
186 | If you disagree with the patch, send a polite response explaining why. If | ||
187 | possible, tell the author what changes need to be made to make the patch | ||
188 | acceptable to you. There is a certain resistance to merging patches which | ||
189 | are opposed by the author and maintainer of the code, but it only goes so | ||
190 | far. If you are seen as needlessly blocking good work, those patches will | ||
191 | eventually flow around you and get into the mainline anyway. In the Linux | ||
192 | kernel, nobody has absolute veto power over any code. Except maybe Linus. | ||
193 | |||
194 | On very rare occasion, you may see something completely different: another | ||
195 | developer posts a different solution to your problem. At that point, | ||
196 | chances are that one of the two patches will not be merged, and "mine was | ||
197 | here first" is not considered to be a compelling technical argument. If | ||
198 | somebody else's patch displaces yours and gets into the mainline, there is | ||
199 | really only one way to respond: be pleased that your problem got solved and | ||
200 | get on with your work. Having one's work shoved aside in this manner can | ||
201 | be hurtful and discouraging, but the community will remember your reaction | ||
202 | long after they have forgotten whose patch actually got merged. | ||