]>
Commit | Line | Data |
---|---|---|
9f95a23c TL |
1 | Submitting Patches to Ceph - Kernel Components |
2 | ============================================== | |
3 | ||
4 | Submission of patches to the Ceph kernel code is subject to the same rules | |
5 | and guidelines as any other patches to the Linux Kernel. These are set out in | |
6 | ``Documentation/process/submitting-patches.rst`` in the kernel source tree. | |
7 | ||
8 | What follows is a condensed version of those rules and guidelines, updated based | |
9 | on the Ceph project's best practices. | |
10 | ||
11 | ||
12 | .. contents:: | |
13 | :depth: 3 | |
14 | ||
15 | ||
16 | Signing contributions | |
17 | --------------------- | |
18 | ||
19 | In order to keep the record of code attribution clean within the source | |
20 | repository, follow these guidelines for signing patches submitted to the | |
21 | project. These definitions are taken from those used by the Linux kernel | |
22 | and many other open source projects. | |
23 | ||
24 | ||
25 | 1. Sign your work | |
26 | ################# | |
27 | ||
28 | To improve tracking of who did what, especially with patches that can | |
29 | percolate to their final resting place in the kernel through several | |
30 | layers of maintainers, we've introduced a "sign-off" procedure on | |
31 | patches that are being emailed around. | |
32 | ||
33 | The sign-off is a simple line at the end of the explanation for the | |
34 | patch, which certifies that you wrote it or otherwise have the right to | |
35 | pass it on as a open-source patch. The rules are pretty simple: if you | |
36 | can certify the below: | |
37 | ||
38 | Developer's Certificate of Origin 1.1 | |
39 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
40 | ||
41 | By making a contribution to this project, I certify that: | |
42 | ||
43 | (a) The contribution was created in whole or in part by me and I | |
44 | have the right to submit it under the open source license | |
45 | indicated in the file; or | |
46 | ||
47 | (b) The contribution is based upon previous work that, to the best | |
48 | of my knowledge, is covered under an appropriate open source | |
49 | license and I have the right under that license to submit that | |
50 | work with modifications, whether created in whole or in part | |
51 | by me, under the same open source license (unless I am | |
52 | permitted to submit under a different license), as indicated | |
53 | in the file; or | |
54 | ||
55 | (c) The contribution was provided directly to me by some other | |
56 | person who certified (a), (b) or (c) and I have not modified | |
57 | it. | |
58 | ||
59 | (d) I understand and agree that this project and the contribution | |
60 | are public and that a record of the contribution (including all | |
61 | personal information I submit with it, including my sign-off) is | |
62 | maintained indefinitely and may be redistributed consistent with | |
63 | this project or the open source license(s) involved. | |
64 | ||
65 | then you just add a line saying :: | |
66 | ||
67 | Signed-off-by: Random J Developer <random@developer.example.org> | |
68 | ||
69 | ||
70 | using your real name (sorry, no pseudonyms or anonymous contributions.) | |
71 | ||
72 | Some people also put extra tags at the end. They'll just be ignored for | |
73 | now, but you can do this to mark internal company procedures or just | |
74 | point out some special detail about the sign-off. | |
75 | ||
76 | If you are a subsystem or branch maintainer, sometimes you need to slightly | |
77 | modify patches you receive in order to merge them, because the code is not | |
78 | exactly the same in your tree and the submitters'. If you stick strictly to | |
79 | rule (c), you should ask the submitter to rediff, but this is a totally | |
80 | counter-productive waste of time and energy. Rule (b) allows you to adjust | |
81 | the code, but then it is very impolite to change one submitter's code and | |
82 | make them endorse your bugs. To solve this problem, it is recommended that | |
83 | you add a line between the last Signed-off-by header and yours, indicating | |
84 | the nature of your changes. While there is nothing mandatory about this, it | |
85 | seems like prepending the description with your mail and/or name, all | |
86 | enclosed in square brackets, is noticeable enough to make it obvious that | |
87 | you are responsible for last-minute changes. Example :: | |
88 | ||
89 | Signed-off-by: Random J Developer <random@developer.example.org> | |
90 | [lucky@maintainer.example.org: struct foo moved from foo.c to foo.h] | |
91 | Signed-off-by: Lucky K Maintainer <lucky@maintainer.example.org> | |
92 | ||
93 | This practise is particularly helpful if you maintain a stable branch and | |
94 | want at the same time to credit the author, track changes, merge the fix, | |
95 | and protect the submitter from complaints. Note that under no circumstances | |
96 | can you change the author's identity (the From header), as it is the one | |
97 | which appears in the changelog. | |
98 | ||
99 | Special note to back-porters: It seems to be a common and useful practise | |
100 | to insert an indication of the origin of a patch at the top of the commit | |
101 | message (just after the subject line) to facilitate tracking. For instance, | |
102 | here's what we see in 2.6-stable :: | |
103 | ||
104 | Date: Tue May 13 19:10:30 2008 +0000 | |
105 | ||
106 | SCSI: libiscsi regression in 2.6.25: fix nop timer handling | |
107 | ||
108 | commit 4cf1043593db6a337f10e006c23c69e5fc93e722 upstream | |
109 | ||
110 | And here's what appears in 2.4 :: | |
111 | ||
112 | Date: Tue May 13 22:12:27 2008 +0200 | |
113 | ||
114 | wireless, airo: waitbusy() won't delay | |
115 | ||
116 | [backport of 2.6 commit b7acbdfbd1f277c1eb23f344f899cfa4cd0bf36a] | |
117 | ||
118 | Whatever the format, this information provides a valuable help to people | |
119 | tracking your trees, and to people trying to trouble-shoot bugs in your | |
120 | tree. | |
121 | ||
122 | ||
123 | 2. When to use ``Acked-by:`` and ``Cc:`` | |
124 | ######################################## | |
125 | ||
126 | The ``Signed-off-by:`` tag indicates that the signer was involved in the | |
127 | development of the patch, or that he/she was in the patch's delivery path. | |
128 | ||
129 | If a person was not directly involved in the preparation or handling of a | |
130 | patch but wishes to signify and record their approval of it then they can | |
131 | arrange to have an ``Acked-by:`` line added to the patch's changelog. | |
132 | ||
133 | ``Acked-by:`` is often used by the maintainer of the affected code when that | |
134 | maintainer neither contributed to nor forwarded the patch. | |
135 | ||
136 | ``Acked-by:`` is not as formal as ``Signed-off-by:``. It is a record that the acker | |
137 | has at least reviewed the patch and has indicated acceptance. Hence patch | |
138 | mergers will sometimes manually convert an acker's "yep, looks good to me" | |
139 | into an ``Acked-by:``. | |
140 | ||
141 | ``Acked-by:`` does not necessarily indicate acknowledgement of the entire patch. | |
142 | For example, if a patch affects multiple subsystems and has an ``Acked-by:`` from | |
143 | one subsystem maintainer then this usually indicates acknowledgement of just | |
144 | the part which affects that maintainer's code. Judgement should be used here. | |
145 | When in doubt people should refer to the original discussion in the mailing | |
146 | list archives. | |
147 | ||
148 | If a person has had the opportunity to comment on a patch, but has not | |
149 | provided such comments, you may optionally add a "Cc:" tag to the patch. | |
150 | This is the only tag which might be added without an explicit action by the | |
151 | person it names. This tag documents that potentially interested parties | |
152 | have been included in the discussion | |
153 | ||
154 | ||
155 | 3. Using ``Reported-by:``, ``Tested-by:`` and ``Reviewed-by:`` | |
156 | ############################################################## | |
157 | ||
158 | If this patch fixes a problem reported by somebody else, consider adding a | |
159 | ``Reported-by:`` tag to credit the reporter for their contribution. This tag should | |
160 | not be added without the reporter's permission, especially if the problem was | |
161 | not reported in a public forum. That said, if we diligently credit our bug | |
162 | reporters, they will, hopefully, be inspired to help us again in the future. | |
163 | ||
164 | A ``Tested-by:`` tag indicates that the patch has been successfully tested (in | |
165 | some environment) by the person named. This tag informs maintainers that | |
166 | some testing has been performed, provides a means to locate testers for | |
167 | future patches, and ensures credit for the testers. | |
168 | ||
169 | ``Reviewed-by:``, instead, indicates that the patch has been reviewed and found | |
170 | acceptable according to the Reviewer's Statement: | |
171 | ||
172 | Reviewer's statement of oversight | |
173 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
174 | ||
175 | By offering my ``Reviewed-by:`` tag, I state that: | |
176 | ||
177 | (a) I have carried out a technical review of this patch to | |
178 | evaluate its appropriateness and readiness for inclusion into | |
179 | the mainline kernel. | |
180 | ||
181 | (b) Any problems, concerns, or questions relating to the patch | |
182 | have been communicated back to the submitter. I am satisfied | |
183 | with the submitter's response to my comments. | |
184 | ||
185 | (c) While there may be things that could be improved with this | |
186 | submission, I believe that it is, at this time, (1) a | |
187 | worthwhile modification to the kernel, and (2) free of known | |
188 | issues which would argue against its inclusion. | |
189 | ||
190 | (d) While I have reviewed the patch and believe it to be sound, I | |
191 | do not (unless explicitly stated elsewhere) make any | |
192 | warranties or guarantees that it will achieve its stated | |
193 | purpose or function properly in any given situation. | |
194 | ||
195 | A ``Reviewed-by`` tag is a statement of opinion that the patch is an | |
196 | appropriate modification of the kernel without any remaining serious | |
197 | technical issues. Any interested reviewer (who has done the work) can | |
198 | offer a ``Reviewed-by`` tag for a patch. This tag serves to give credit to | |
199 | reviewers and to inform maintainers of the degree of review which has been | |
200 | done on the patch. ``Reviewed-by:`` tags, when supplied by reviewers known to | |
201 | understand the subject area and to perform thorough reviews, will normally | |
202 | increase the likelihood of your patch getting into the kernel. | |
203 | ||
204 | ||
205 | Preparing and sending patches | |
206 | ----------------------------- | |
207 | ||
208 | For the kernel client, patches are expected to be emailed directly to the | |
209 | email list ``ceph-devel@vger.kernel.org`` (note: *not* ``dev@ceph.io``) and reviewed | |
210 | in the email list. | |
211 | ||
212 | The best way to generate a patch for manual submission is to work from | |
213 | a Git checkout of the Ceph kernel client (kernel modules) repository located at | |
214 | https://github.com/ceph/ceph-client. You can then generate patches | |
215 | with the 'git format-patch' command. For example, | |
216 | ||
217 | .. code-block:: bash | |
218 | ||
219 | $ git format-patch HEAD^^ -o mything | |
220 | ||
221 | will take the last two commits and generate patches in the mything/ | |
222 | directory. The commit you specify on the command line is the | |
223 | 'upstream' commit that you are diffing against. Note that it does | |
224 | not necessarily have to be an ancestor of your current commit. You | |
225 | can do something like | |
226 | ||
227 | .. code-block:: bash | |
228 | ||
229 | $ git checkout -b mything | |
230 | # ... do lots of stuff ... | |
231 | $ git fetch | |
232 | # ...find out that origin/unstable has also moved forward... | |
233 | $ git format-patch origin/unstable -o mything | |
234 | ||
235 | and the patches will be against origin/unstable. | |
236 | ||
237 | The ``-o`` dir is optional; if left off, the patch(es) will appear in | |
238 | the current directory. This can quickly get messy. | |
239 | ||
240 | You can also add ``--cover-letter`` and get a '0000' patch in the | |
241 | mything/ directory. That can be updated to include any overview | |
242 | stuff for a multipart patch series. If it's a single patch, don't | |
243 | bother. | |
244 | ||
245 | Make sure your patch does not include any extra files which do not | |
246 | belong in a patch submission. Make sure to review your patch -after- | |
247 | generated it with ``diff(1)``, to ensure accuracy. | |
248 | ||
249 | If your changes produce a lot of deltas, you may want to look into | |
250 | splitting them into individual patches which modify things in | |
251 | logical stages. This will facilitate easier reviewing by other | |
252 | kernel developers, very important if you want your patch accepted. | |
253 | There are a number of scripts which can aid in this. | |
254 | ||
255 | The ``git send-email`` command make it super easy to send patches | |
256 | (particularly those prepared with git format patch). It is careful to | |
257 | format the emails correctly so that you don't have to worry about your | |
258 | email client mangling whitespace or otherwise screwing things up. It | |
259 | works like so: | |
260 | ||
261 | .. code-block:: bash | |
262 | ||
263 | $ git send-email --to ceph-devel@vger.kernel.org my.patch | |
264 | ||
265 | for a single patch, or | |
266 | ||
267 | .. code-block:: bash | |
268 | ||
269 | $ git send-email --to ceph-devel@vger.kernel.org mything | |
270 | ||
271 | to send a whole patch series (prepared with, say, git format-patch). | |
272 | ||
273 | ||
274 | No MIME, no links, no compression, no attachments. Just plain text | |
275 | ------------------------------------------------------------------ | |
276 | ||
277 | Developers need to be able to read and comment on the changes you are | |
278 | submitting. It is important for a kernel developer to be able to | |
279 | "quote" your changes, using standard e-mail tools, so that they may | |
280 | comment on specific portions of your code. | |
281 | ||
282 | For this reason, all patches should be submitting e-mail "inline". | |
283 | WARNING: Be wary of your editor's word-wrap corrupting your patch, | |
284 | if you choose to cut-n-paste your patch. | |
285 | ||
286 | Do not attach the patch as a MIME attachment, compressed or not. | |
287 | Many popular e-mail applications will not always transmit a MIME | |
288 | attachment as plain text, making it impossible to comment on your | |
289 | code. A MIME attachment also takes Linus a bit more time to process, | |
290 | decreasing the likelihood of your MIME-attached change being accepted. | |
291 | ||
292 | Exception: If your mailer is mangling patches then someone may ask | |
293 | you to re-send them using MIME. | |
294 | ||
295 | ||
296 | Style Guide | |
297 | ----------- | |
298 | ||
299 | The Linux Kernel has coding style conventions, which are set forth in | |
300 | ``Documentation/process/coding-style.rst``. Please adhere to these conventions. |